-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
FIX: clean up unit conversion unpacking of data, particularly for dates and pandas series #11664
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 all 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 |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ | |
|
|
||
| import datetime | ||
| import logging | ||
| import numbers | ||
| import warnings | ||
|
|
||
| import numpy as np | ||
|
|
||
|
|
@@ -1477,9 +1479,14 @@ def have_units(self): | |
| return self.converter is not None or self.units is not None | ||
|
|
||
| def convert_units(self, x): | ||
|
Contributor
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. This seems to be converting anything numlike, which includes things like
Member
Author
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. Great, I’ll check vs pint when I look at the other comments above. Thanks for the reminder...
Contributor
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. Ok, I guess I still don't understand how all of this works. 😉 So |
||
| # If x is already a number, doesn't need converting | ||
| # If x is already a number, doesn't need converting, but | ||
| # some iterables need to be massaged a bit... | ||
| if munits.ConversionInterface.is_numlike(x): | ||
| return x | ||
| if isinstance(x, list) or isinstance(x, numbers.Number): | ||
|
Member
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. What is the rationale for returning a list as-is instead of running it through |
||
| return x | ||
| if issubclass(type(x), np.ma.MaskedArray): | ||
|
Member
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. Why is this using |
||
| return np.ma.asarray(x) | ||
| return np.asarray(x) | ||
|
Member
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. Is the point of this whole addition above to short-circuit some of what would otherwise happen in
Contributor
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. it seems worse, because calling |
||
|
|
||
| if self.converter is None: | ||
| self.converter = munits.registry.get_converter(x) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,6 +419,11 @@ def date2num(d): | |
| return _dt64_to_ordinalf(d) | ||
| if not d.size: | ||
| return d | ||
| if hasattr(d, 'dtype'): | ||
| if ((isinstance(d, np.ndarray) and | ||
| np.issubdtype(d.dtype, np.datetime64)) or | ||
| isinstance(d, np.datetime64)): | ||
| return _dt64_to_ordinalf(d) | ||
|
Member
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. What case is this whole block covering? Again, I'm confused. |
||
| return _to_ordinalf_np_vectorized(d) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -616,6 +616,16 @@ def tz_convert(*args): | |
| _test_date2num_dst(pd.date_range, tz_convert) | ||
|
|
||
|
|
||
| def test_dateboxplot_pandas(pd): | ||
| # smoke test that this doesn't fail. | ||
| data = np.random.rand(5, 2) | ||
| years = pd.date_range('1/1/2000', | ||
| periods=2, freq=pd.DateOffset(years=1)).year | ||
| # Does not work | ||
|
Member
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. Another place I am confused. Is the comment obsolete? |
||
| plt.figure() | ||
| plt.boxplot(data, positions=years) | ||
|
|
||
|
|
||
| def _test_rrulewrapper(attach_tz, get_tz): | ||
| SYD = get_tz('Australia/Sydney') | ||
|
|
||
|
|
||
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.
I don't see any point in removing the "copy=False" kwarg. It saves time, and it is logically what is wanted here. A copy was already made via the
column_stackcall, so nothing done to coords will affect X or Y.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.
Not sure. Changed back