Skip to content

add initial pandas HDF fileformat #23

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Feb 3, 2022
Merged

add initial pandas HDF fileformat #23

merged 9 commits into from
Feb 3, 2022

Conversation

wkerzendorf
Copy link
Member

The package was exactly what I'm looking for (for years I had some hack in TARDIS to do that). However, we store our reference files in the pandas HDF format (a specialized HDF5 format). I've added a simple initial implementation for the HDF format. If this is something that you would consider adding then I will clean it up and add tests

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from 7438d3a to ab6661a Compare October 19, 2020 22:06
@wkerzendorf
Copy link
Member Author

@astrofrog I added the code and the tests. I would add some more descriptions to the Readme. Is there anything else you would like for this PR?

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch 2 times, most recently from 9822b71 to 5ca61e4 Compare October 21, 2020 07:57
astrofrog
astrofrog approved these changes Oct 21, 2020
View reviewed changes
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, thanks! Before I merge can you add an entry to the changelog under section 0.4?

@wkerzendorf
Copy link
Member Author

@astrofrog I'd like to rename the format pd_hdf if that is alright with you. In the future, we can just add pd_excel or other formats that people would like to save as.

@wkerzendorf
Copy link
Member Author

@astrofrog ready to merge.

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from 99a7ae2 to 27f541b Compare October 29, 2020 14:33
@wkerzendorf
Copy link
Member Author

@astrofrog how do I add the dependencies now with tox enabled? I tried - but failed.

astrofrog
astrofrog requested changes Oct 31, 2020
View reviewed changes
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.

A couple of small requests below - add pandas and pytables under here:

https://github.com/astropy/pytest-arraydiff/blob/master/setup.cfg#L31

I'll open a separate PR to make those optional dependencies along with astropy.

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch 2 times, most recently from 7918451 to 8dc6eb1 Compare November 3, 2020 22:09
@wkerzendorf
Copy link
Member Author

@astrofrog okay - everything works - except macos because I need the HDF5 libraries. You guys don't use conda anymore I guess. How do I install these libraries now?

@pllim
Copy link
Member

pllim commented Dec 2, 2020

Try wheels? https://docs.h5py.org/en/stable/build.html#wheels

wkerzendorf
wkerzendorf commented Dec 4, 2020
View reviewed changes
@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from a6206ba to a646551 Compare December 9, 2020 19:59
@wkerzendorf
Copy link
Member Author

@astrofrog they all pass - can we get a merge please 😄

Base automatically changed from master to main March 9, 2021 23:46
@wkerzendorf
Copy link
Member Author

@astrofrog - is there anything missing for a merge?

@wkerzendorf
Copy link
Member Author

@astrofrog I think I added everything - is there anything else?

@epassaro
Copy link

epassaro commented Nov 4, 2021

@wkerzendorf you can try removing my patch, maybe it's not necessary anymore.

@pllim pllim added this to the v0.5.0 milestone Dec 31, 2021
@pllim pllim modified the milestones: v0.5.0, v0.6.0 Jan 13, 2022
astrofrog
astrofrog requested changes Jan 20, 2022
View reviewed changes
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.

This looks good to me! Just a couple of minor requests below - also can you rebase to get rid of the merge commit? Thanks!

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from cd4c1b8 to 7438d3a Compare January 20, 2022 18:01
@astrofrog
Copy link
Member

It looks like the changelog entry is now missing?

@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from 7438d3a to b2d595a Compare January 20, 2022 18:13
@wkerzendorf wkerzendorf force-pushed the add_pdhdf_file_format branch from b2d595a to 1e12df4 Compare January 20, 2022 18:15
@wkerzendorf
Copy link
Member Author

@astrofrog the changelog entry is back. I think it should be good to go.

astrofrog
astrofrog approved these changes Feb 3, 2022
View reviewed changes
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, thanks!

@astrofrog astrofrog merged commit 26f1d67 into astropy:main Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pllim pllim pllim left review comments

@astrofrog astrofrog astrofrog approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@wkerzendorf @pllim @epassaro @astrofrog