GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Implemented fix to issue 196 on github for log=True and histtype='step' by keflavich · Pull Request #1029 · matplotlib/matplotlib

Conversation

@keflavich

@keflavich keflavich commented Jul 20, 2012

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@keflavich
Implemented fix to issue 196 on github for log=True and histtype='step'
90f8807
WeatherGod
WeatherGod reviewed Jul 20, 2012
View reviewed changes
Comment thread lib/matplotlib/axes.py Outdated

@WeatherGod WeatherGod Jul 20, 2012

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@keflavich
fixed the minimum specification in ax.hist as per @WeatherGod's comment;
f888a26
only want to set min this way if log is specified.  Also, added case for
normed vs not
@keflavich

keflavich commented Jul 20, 2012

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@WeatherGod

WeatherGod commented Jul 21, 2012

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@keflavich

keflavich commented Jul 21, 2012

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" <
[email protected]>
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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@WeatherGod

WeatherGod commented Jul 21, 2012

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mdboom

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@dmcdougall

dmcdougall commented Jan 18, 2013

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ghost ghost assigned dmcdougall Jan 18, 2013
@keflavich

keflavich commented 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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@dmcdougall dmcdougall mentioned this pull request Jan 19, 2013
Fix hist for log=True and histtype='step' #1684
Merged
@dmcdougall

dmcdougall commented Jan 27, 2013

Copy link
Copy Markdown
Member

Resolved in #1684.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@dmcdougall dmcdougall closed this Jan 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@dmcdougall dmcdougall

Labels

None yet

Projects

None yet

Milestone

v1.2.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@keflavich @WeatherGod @mdboom @dmcdougall @piti118
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.