-
Notifications
You must be signed in to change notification settings - Fork 224
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
Use np.asarray to convert a 1-D array to datetime type in array_to_datetime #2481
Conversation
A new failing test after the above fix:
It's caused by the incompatible changes in https://pandas.pydata.org/docs/whatsnew/v2.0.0.html#datetimes-are-now-parsed-with-a-consistent-format.
In our case, we're passing datetimes in different formats, but pandas tries to parse all datetimes using a consistent format |
pygmt/pygmt/clib/conversion.py Line 329 in aaa0ca0
format="mixed" is not available for pandas < 2.0.0, so we have to pin pandas to >=2.0.0 or check the pandas version.array_to_datetime support **kwargs so that we can pass any keyword arguments to pd.to_dataframe . But the array_to_datetime function is only internally used by PyGMT, so users who pass a list of datetimes with mixed formats will still see the same error.
|
The Lines 797 to 805 in aaa0ca0
Please note that before calling In the Lines 744 to 752 in aaa0ca0
Thus, if a list/vector can't be processed by In short, I think we should use |
... "5/1/2018", | ||
... "Jun 05, 2018", | ||
... "2018/07/02", |
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.
These three types of datetime inputs are no longer supported. Actually, they're never supported because they won't be recognized as valid datetime strings in the _check_dtype_and_dim
function.
97abc5f
to
2ac723e
Compare
I've made the change in commit 608e29b and also use the same function in |
np.asarray
to convert a 1-D array to datetime type in array_to_datetime
np.asarray
to convert a 1-D array to datetime type in array_to_datetime""" | ||
return pd.to_datetime(array) | ||
return np.asarray(array, dtype=np.datetime64) |
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 feel like there was a reason we used pd.to_datetime
instead of np.asarray
in #464, maybe to support funny dates like 'Jun 05, 2018', but since the tests pass and you mentioned in https://github.com/GenericMappingTools/pygmt/pull/2481/files#r1158514955 that it's not actually supported by _check_dtype_and_dim
, then it should be fine 🤞
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 feel like there was a reason we used
pd.to_datetime
instead ofnp.asarray
in #464, maybe to support funny dates like 'Jun 05, 2018',
In the Plotting datetime charts tutorial, we probably need to add a paragraph or a subsection to explain that, for non-ISO datetimes like 'Jun 05, 2018', people should use pd.to_datetime
to process it first before passing it to PyGMT.
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.
Under https://www.pygmt.org/v0.9.0/tutorials/advanced/date_time_charts.html#generating-an-automatic-region, pd.to_datetime
is used to convert dates like "20220712" into a recognizable format. Maybe add a few sentences somewhere there?
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 added a sentence under the "Using ISO Format" section (ada5616).
For mysterious reasons, suddenly |
Related to pydata/xarray#7716. |
Yeah, looks like they did a repodata patch at conda-forge/conda-forge-repodata-patches-feedstock#426 for xarray versions 2022.10.0 to 2023.2.0 at (i.e. the CalVer versions), but they missed the older SemVer versions (e.g. 0.19.0) so we'll need to wait for conda-forge/conda-forge-repodata-patches-feedstock#429 🤷 |
Co-authored-by: Michael Grund <[email protected]>
Co-authored-by: Michael Grund <[email protected]>
Description of proposed changes
Address #2480.
The initial purpose of this PR is to fix the failing doctests as mentioned in #2480, but in comment #2481 (comment) I decided to refactor the
array_to_datetime
function.This PR fixes the following failing test:
It's caused by incompatible changes as documented in
https://pandas.pydata.org/docs/whatsnew/v2.0.0.html#construction-with-datetime64-or-timedelta64-dtype-with-unsupported-resolution.
Fixes #
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version