-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Don't convert numbers plotted on an axis with units #10206
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1498,6 +1498,10 @@ def have_units(self): | |
| return self.converter is not None or self.units is not None | ||
|
|
||
| def convert_units(self, x): | ||
| # If x is already a number, doesn't need converting | ||
| if munits.ConversionInterface.is_numlike(x): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the assigned converter deals with the floats. This change means that a user can never create a converter that accepts I guess I'm OK w/ that restriction, and I don't think it harms any of our current converters. But, I'm not 100% sure what the rest of the world is doing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW pinging @tacaswell, @anntzer and @story645 as they are working on categoricals where unit handling has come up a lot There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, you make a good point. If the decision is to allow plotting numbers on an axis with units, then this PR is the way to go. (In principle I'm -1 on that, but I wasn't on the call so I'm guessing there's good reasons for allowing it?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are good reasons for both. @efiring 's example was: imagine you plot five categories, and then want a line at x=0.5. Thats hard to do if we have locked out x to be only categories. I'm not 100% convinced we can't come up w/ a way to let the user force the units, but still retain the lockout by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I wasn't really "working on categoricals", just trying to write some version that's reasonably robust and doesn't do too silly things (which is quite hard) :-)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a decision on supporting mixed types in categorical? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just strings, according to #10212 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Axvline plots a line at an x in data coordinates. If you want that half-way between two categorical values, you have to be able to specify that location in numeric data coordinates. If you know how categoricals are mapped to numbers, and you are allowed to plot in numeric data coordinates, this is easy. Fundamentally, all plotting is done in floating point coordinates. Plotting something in "units" should not prevent the user from positioning something directly in the numbers to which those units map--and from doing so efficiently. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding your example, it should be well-documented--if it isn't already--that categoricals map sequentially to integers starting from zero. Knowing that, the user can easily calculate that the point half-way between your '2' and '3' (which more realistically might be 'dogs' and 'cats' or "instrument 872" and "instrument 253") is 1.5. |
||
| return x | ||
|
|
||
| if self.converter is None: | ||
| self.converter = munits.registry.get_converter(x) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,10 @@ def __getattr__(self, attr): | |
| return getattr(self.magnitude, attr) | ||
|
|
||
| def __getitem__(self, item): | ||
| return Quantity(self.magnitude[item], self.units) | ||
| if iterable(self.magnitude): | ||
| return Quantity(self.magnitude[item], self.units) | ||
| else: | ||
| return Quantity(self.magnitude[item], self.units) | ||
|
||
|
|
||
| def __array__(self): | ||
| return np.asarray(self.magnitude) | ||
|
|
||