FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Implemented fix to issue 196 on github for log=True and histtype='step' by keflavich · Pull Request #1029 · matplotlib/matplotlib · GitHub

Repository navigation

Implemented fix to issue 196 on github for log=True and histtype='step' - #1029

Closed
keflavich wants to merge 2 commits into
matplotlib:masterfrom
keflavich:fix_loghist
Closed

keflavich wants to merge 2 commits into
matplotlib:masterfrom
keflavich:fix_loghist

Conversation

Copy link
Copy Markdown
Contributor

A solution was proposed in Issue #196. This pull request implements that proposed solution.

Comment thread lib/matplotlib/axes.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

wouldn't we only want to do this for 'log'? Also, since bins is a list and the operation is just a simple filter, let's go with that route.

Add a comment for the rationale for the filtering and also for the 0.1 multiplication.

only want to set min this way if log is specified.  Also, added case for
normed vs not

Copy link
Copy Markdown
Contributor Author

@WeatherGod - I think you're right, you only want to do it this way for log. You also want different minima for normed vs not.

Copy link
Copy Markdown
Member

Why 0.1? What is special about that number versus any other number? Could it accidentially be related to the fact that the default number of bins is 10 (I think)? If so, then we can't rely on that.

Copy link
Copy Markdown
Contributor Author

No, 0.1 corresponds to one tick label unit for log plots assuming log base
10.
On Jul 20, 2012 6:26 PM, "Benjamin Root" <
reply@reply.github.com>
wrote:

Why 0.1? What is special about that number versus any other number?
Could it accidentially be related to the fact that the default number of
bins is 10 (I think)? If so, then we can't rely on that.


Reply to this email directly or view it on GitHub:
#1029 (comment)

Copy link
Copy Markdown
Member

ok, so probably the only thing left is to add a test for this. If you need help with that, just let me know.

mdboom commented Jul 30, 2012

Copy link
Copy Markdown
Member

It should probably use the log's actual base -- it's 10 by default, but it could be anything.

Copy link
Copy Markdown
Member

This should go in v1.2.x, it's a fix. I'm milestoning it as such.

@keflavich Are you able to add a test for this? If not I can fork your branch and move this forward.

ghost assigned dmcdougall Jan 18, 2013

Copy link
Copy Markdown
Contributor Author

Best if you go ahead; I'll not have a chance for at least a week

Copy link
Copy Markdown
Member

Resolved in #1684.

dmcdougall closed this Jan 27, 2013
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL