Masking invalid x and/or weights in hist#9706
Conversation
| @@ -6061,6 +6061,46 @@ def hist(self, x, bins=None, range=None, density=None, weights=None, | |||
| bin_range = range | |||
| del range | |||
There was a problem hiding this comment.
There is a namespace conflict here. range is both the range of histogram and the python function. If I do just do a quick and dirty example:
del range
range(np.arange(100))
I get the below error message:
---------------------------------------------------------------------------
NameError Traceback (most recent call last)
<ipython-input-21-0eb074616bf8> in <module>()
----> 1 del range
2 range(np.arange(100))
NameError: name 'range' is not defined
This seems similar to the errors generated in the tests, which comes from using range to refer to the built-in python function on line 6171.
| bin_range = range | ||
| del range | ||
|
|
||
| def _normalize_input(inp, ename='input'): |
There was a problem hiding this comment.
I think the problem is that this function has disappeared on the master branch, so I think another approach to the problem is needed.
|
I've tidied this up a bit - using masked arrays is not working at all with the rest of the |
|
Yeah, I think |
|
🤦♀️ turns out the issue was PEBKAC and not the masked arrays... should be fine now |
|
(this still needs a test and a rebase, but I think the code is fine now) |
dstansby
force-pushed
the
hist-masking
branch
from
974904d to
17a01b3
Compare
dstansby
force-pushed
the
hist-masking
branch
from
17a01b3 to
9ee390f
Compare
|
Looks like you also need to modify the docstring to note the support for masked arrays and invalid values, and also note the support for masked arrays in the whats_new entry. |
dstansby
force-pushed
the
hist-masking
branch
from
9ee390f to
d9c49aa
Compare
Tidying up the data prep work in hist. Mask invalid entries in x and weights before plotting histograms. Clean up hist masking code Fix masking
dstansby
force-pushed
the
hist-masking
branch
from
d9c49aa to
c4b9364
Compare
jklymak
left a comment
jklymak
left a comment
There was a problem hiding this comment.
This looks good. I'm a little curious what masked values do to density, but I guess thats really in np.histogram, not here.
dstansby
added
the
Release critical
tacaswell
removed
the
Release critical
efiring
dismissed
their stale review
It looks like this is actually not supporting masked arrays as it says it is.
|
Due to some on-going discussion about what to do in the case of missing data (and how the handle the averaging). |
jklymak
dismissed
their stale review
As above - this needs a bit more discussion
efiring
left a comment
efiring
left a comment
There was a problem hiding this comment.
- the docstring is contradictory.
- masked arrays and nans are being passed to np.histogram
- there are arguments as to how bad values should be handles
|
Going to close this since it's been a while - I'll leave my branch around for a bit if anyone thinks it's worthwhile picking up. |
7 participants
A rebase of #7133, fixes #6483.