Skip to content

add initial pandas HDF fileformat#23

Merged
astrofrog merged 9 commits intoastropy:mainfrom
wkerzendorf:add_pdhdf_file_format
Feb 3, 2022
Merged

add initial pandas HDF fileformat#23
astrofrog merged 9 commits intoastropy:mainfrom
wkerzendorf:add_pdhdf_file_format

Conversation

@wkerzendorf
Copy link
Copy Markdown
Member

@wkerzendorf wkerzendorf commented Oct 17, 2020

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
Copy Markdown
Member Author

wkerzendorf commented Oct 19, 2020

@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
Copy Markdown
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
Copy Markdown
Member Author

wkerzendorf commented Oct 22, 2020

@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
Copy Markdown
Member Author

wkerzendorf commented Oct 23, 2020

@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
Copy Markdown
Member Author

wkerzendorf commented Oct 29, 2020

@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
Copy Markdown
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
Copy Markdown
Member Author

wkerzendorf commented Dec 2, 2020

@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
Copy Markdown
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
Copy Markdown
Member Author

wkerzendorf commented Dec 9, 2020

@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
Copy Markdown
Member Author

wkerzendorf commented Apr 26, 2021

@astrofrog - is there anything missing for a merge?

@wkerzendorf
Copy link
Copy Markdown
Member Author

wkerzendorf commented Nov 4, 2021

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

@epassaro
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Member

astrofrog commented Jan 20, 2022

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
Copy Markdown
Member Author

wkerzendorf commented Jan 20, 2022

@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
Copy Markdown
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