Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ValueError being raised when plotting hist and hexbin on empty dataset (Fix #3886) #4119

Merged
merged 7 commits into from Mar 30, 2015
Merged

Fix ValueError being raised when plotting hist and hexbin on empty dataset (Fix #3886) #4119

merged 7 commits into from Mar 30, 2015

Conversation

umairidris
Copy link

Fix #3886

@umairidris umairidris changed the title Fix ValueError being raised when plotting hist and hexbin on empty dataset (Issue #3886) Fix ValueError being raised when plotting hist and hexbin on empty dataset Feb 17, 2015
@@ -482,6 +482,9 @@ def test_hexbin_extent():

ax.hexbin(x, y, extent=[.1, .3, .6, .7])

def test_hexbin_empty():
# From #3886: creating hexbin from empty dataset raises ValueError
ax.hexbin([], [])
Copy link
Member

Choose a reason for hiding this comment

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

You need to create the Axes first. Also, I think you will want to use the @cleanup decorator.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! This is now done via commit cb8539f.

@tacaswell tacaswell added this to the next point release milestone Feb 18, 2015
@tacaswell
Copy link
Member

Other than the testing issues Eric pointed out, this looks good.

Could you also check the docstrings to make sure there are not any references left to this restriction?

@@ -5607,7 +5606,7 @@ def hist(self, x, bins=10, range=None, normed=False, weights=None,
# basic input validation
flat = np.ravel(x)
if len(flat) == 0:
raise ValueError("x must have at least one data point")
return [], [], cbook.silent_list('No Patches')
Copy link
Member

Choose a reason for hiding this comment

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

Do we want do draw no patches or patches of zero size? On consideration I think patches of zero size might be a better option here so client code which expects a non-zero length list does not have to check that it is true. For example if the user passes in bin edges, they should get back a list of zeros telling them that there are no counts in any of the bins.

Copy link
Member

Choose a reason for hiding this comment

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

@efiring @mdboom @WeatherGod @pelson Thoughts, I think that this is an important API choice that needs to be made.

@efiring
Copy link
Member

efiring commented Feb 18, 2015

I see that np.histogram returns an array of zeros and an array of boundaries when given an empty input array. I think we should follow that logic, which implies that we should be drawing zero-height (or width) patches.

@WeatherGod
Copy link
Member

Yeah, I think that was one of my contributions to numpy... so, I certainly
would agree with that assessment.

Have we fixed issues with autoscaling the axis when there are empty patches?

Ben Root

On Wed, Feb 18, 2015 at 2:31 PM, Eric Firing notifications@github.com
wrote:

I see that np.histogram returns an array of zeros and an array of
boundaries when given an empty input array. I think we should follow that
logic, which implies that we should be drawing zero-height (or width)
patches.


Reply to this email directly or view it on GitHub
#4119 (comment)
.

@umairidris
Copy link
Author

I have attempted to address the feedback (via 161eb17):

  • hist now is consistent with the result of np.histogram (in fact uses np.histogram's return value). Thus, if a user passes in bin edges they now get an array of zeroes back accordingly.
  • There is now (a single) zero-height and zero-width patch being drawn. This should also address the concern of client code checking/requiring there there to be at least 1 patch.

@tacaswell
Copy link
Member

I think it should return the correct number of patches, drawn at the correct places. The application I am thinking of is if you doing something like

vals, edges, patches = ax.hist([], bins=15, bottoms=range(N))
for v, p in zip(vals, patches):
    f(v, p)

Also, putting the patch at 0, 0 is probably not right thing to do because it can mess up auto-limiting.

I strongly suspect that the right thing to do is to just let the results from the np.hist([], ...) propagate through the rest of the code as-is.

@umairidris
Copy link
Author

I've let the results of np.hist propagate through the code (after some minor adjustments to part of the code that needed non empty input) and set the patch to be zero size (via width of bar patch and linewidth of fill patch). Please have a look!

@@ -5761,6 +5762,8 @@ def hist(self, x, bins=10, range=None, normed=False, weights=None,
for m, c in zip(n, color):
if bottom is None:
bottom = np.zeros(len(m), np.float)
if input_empty:
width = 0
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

As per @efiring 's comment: "I think we should follow that logic, which implies that we should be drawing zero-height (or width) patches."

Without setting width to 0, the patches for bar have size 0.1 and thus an empty graph (as desired) is not drawn.

Before (width 0.1, non empty graph):
bar_width_nonzero

After (width 0, empty graph):
bar_width_zero

However, currently the code still calculates the original width of 0.1 and then overwrites it, I will send a commit which moves it into a more appropriate place (where width, dw and boffset are originally calculated) in a future commit if this is the desired result.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Done via 40cf945

@tacaswell
Copy link
Member

We probably do not need the pdf and svg tests in this case. Can you please add extensions=['png'] to the kwarg of the decorator and re-base this branch to remove the pdf/svg from history?

@umairidris
Copy link
Author

We probably do not need the pdf and svg tests in this case. Can you please add extensions=['png'] to the kwarg of the decorator and re-base this branch to remove the pdf/svg from history?

Done.

@umairidris
Copy link
Author

There were far too many useless commits in this PR (PEP8 fixes, comment/docstring changes, etc) given the relative size of the change (forgive my inexperience, this was my first PR). I've squashed some of the unnecessary commits together via rebase for a cleaner merge with master. Thanks!

@tacaswell
Copy link
Member

@jenshnielsen This is having strange issue with the docs, can you take a look?

@jenshnielsen
Copy link
Member

Looks like pip install of the doc dependencies just failed (the connection was cut in the middle of the IPython download). That really should make the build fail earlier. I have restarted the docs build


# Massage 'x' for processing.
# NOTE: Be sure any changes here is also done below to 'weights'
if isinstance(x, np.ndarray) or not iterable(x[0]):
if input_empty:
x = np.array([[]])
Copy link
Member

Choose a reason for hiding this comment

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

Why a 2D array instead of 1D?

Copy link
Member

Choose a reason for hiding this comment

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

never mind, I see why.

@umairidris umairidris changed the title Fix ValueError being raised when plotting hist and hexbin on empty dataset Fix ValueError being raised when plotting hist and hexbin on empty dataset (Fix ##3886) Mar 23, 2015
@umairidris umairidris changed the title Fix ValueError being raised when plotting hist and hexbin on empty dataset (Fix ##3886) Fix ValueError being raised when plotting hist and hexbin on empty dataset (Fix #3886) Mar 23, 2015
tacaswell added a commit that referenced this pull request Mar 30, 2015
ENH : Allow empty input to hist and hexbin 

closes #3886
@tacaswell tacaswell merged commit b7df247 into matplotlib:master Mar 30, 2015
@tacaswell
Copy link
Member

@umairidris Thank you!

@umairidris umairidris deleted the hist_hexbin_empty branch April 12, 2015 03:35
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.

behavior when plotting no data
5 participants