-
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
Refactor info to allow datetime inputs from xarray.Dataset and pandas.DataFrame tables #619
Conversation
output = info(table=table) | ||
expected_output = ( | ||
"<vector memory>: N = 5 <10/15> <2020-01-01T00:00:00/2020-01-05T00:00:00>\n" | ||
) | ||
assert output == expected_output |
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.
Note that this test currently fails, giving what looks like a UNIX timestamp rather than an ISO datetime output:
E AssertionError: assert '<vector memo...1578182400>\n' == '<vector memo...5T00:00:00>\n'
E - <vector memory>: N = 5 <10/15> <2020-01-01T00:00:00/2020-01-05T00:00:00>
E + <vector memory>: N = 5 <10/15> <1577836800/1578182400>
Will need to investigate, but any idea why this is happening? Or is this expected behaviour?
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.
GMT_Put_Vector started to accept datetime strings in PR GenericMappingTools/gmt#3396. In this implementation, all datetime strings are converted to double
internally.
Looks like a GMT API bug to me.
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.
So the inputs are read correctly here, we can pass in datetime types using the GMT_DATETIME
enum. The output is also the correct UNIX timestamp range, but I doubt it's something users would expect.
I suppose we can xfail
this for 6.1.1, but would prefer to have this reported and/or fixed in GMT master first (i.e. for 6.2.0) Edit: issue opened at GenericMappingTools/gmt#4241. A workaround might be to use np.datetime64(1577836800, 's')
to get numpy.datetime64('2020-01-01T00:00:00')
, but this isn't a nice solution.
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 suppose we can
xfail
this for 6.1.1,
Yes to me.
Co-Authored-By: Dongdong Tian <[email protected]>
Bumps [pygmt](https://github.com/GenericMappingTools/pygmt) from 0.2.0 to 0.2.0+53.gc7c5e. - [Release notes](https://github.com/GenericMappingTools/pygmt/releases) - [Changelog](https://github.com/GenericMappingTools/pygmt/blob/master/doc/changes.rst) - [Commits](GenericMappingTools/pygmt@v0.2.0...v0.2.0-53-gc7c5e) Had to fix a broken test by using a pandas.DataFrame input to `pygmt.info` instead of a pandas.Series, because of GenericMappingTools/pygmt#619. Also pinned intake-geopandas to the v0.2.4 tag, same version, just using the wheel now.
Description of proposed changes
In order to support datetime inputs into
pygmt.info
, this PR changes the backend mechanism from usinglib.virtualfile_from_matrix()
(which only supports single non-datetime dtypes) to usinglib.virtualfile_from_vectors()
(which supports datetime inputs as of #464).Note: While datetime inputs can be passed into
info
now, the resulting outputs (UNIX timestamp instead of ISO datetime) might not make sense due to the issue at GenericMappingTools/gmt#4241.Coincidentally, this refactor also enables the following:
xarray.Dataset
inputs! Note: Only forxarray.Dataset
s made up of 1Dxarray.DataArrays
s (e.g. what you might get from usingxarray.Dataset.from_dataframe
).Addresses #597, will be fixed once GenericMappingTools/gmt#4241 is resolved.
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.