Skip to content
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

Improve Table to/from pandas #8247

Merged
merged 17 commits into from
Dec 14, 2018
Merged

Improve Table to/from pandas #8247

merged 17 commits into from
Dec 14, 2018

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Dec 7, 2018

This improves the support for converting Table to and from pandas:

  • Works (to the extent possible) with all mixin columns by encoding (flattening) columns as necessary to plain columns.
  • Supports round-tripping Time and TimeDelta via numpy datetime64 and timedelta64.
  • Handles index columns (both in pandas and the primary index in Table if it exists) for to_pandas().

To do:

  • Handle index in from_pandas().

See also #8246 that came out of this work.

Closes #5081

@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

Merging #8247 into master will increase coverage by 0.01%.
The diff coverage is 98.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8247      +/-   ##
==========================================
+ Coverage   86.91%   86.92%   +0.01%     
==========================================
  Files         383      384       +1     
  Lines       57889    57995     +106     
  Branches     1056     1060       +4     
==========================================
+ Hits        50313    50412      +99     
- Misses       6962     6968       +6     
- Partials      614      615       +1
Impacted Files Coverage Δ
astropy/time/formats.py 97.5% <100%> (+0.03%) ⬆️
astropy/table/table.py 93.19% <98.48%> (+0.26%) ⬆️
astropy/utils/data.py 80.7% <0%> (-0.23%) ⬇️
astropy/uncertainty/core.py 94.35% <0%> (-0.05%) ⬇️
astropy/table/__init__.py 100% <0%> (ø) ⬆️
astropy/units/core.py 95.43% <0%> (ø) ⬆️
astropy/io/misc/asdf/connect.py 77.14% <0%> (ø)
astropy/io/ascii/src/tokenizer.c 87.58% <0%> (+0.8%) ⬆️
astropy/io/misc/asdf/tags/table/table.py 92% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e720bee...821946f. Read the comment docs.

@taldcroft
Copy link
Member Author

Rebased and passing all tests now.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taldcroft - This looks good to me, but just one question - is YAML really needed since we aren't actually saving the YAML metadata, but just using the serialization machinery?

@taldcroft
Copy link
Member Author

@astrofrog - good catch! I was just blindly copying precedent in HDF5 or ECSV, but in fact there is no YAML dependency. See 821946f.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now provided tests pass!

@taldcroft taldcroft merged commit 52d1c24 into astropy:master Dec 14, 2018
@taldcroft taldcroft deleted the to-pandas branch December 14, 2018 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants