Skip to content

fix #702 : fixed bug when writing metadata using HDF format #740

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

Merged
merged 1 commit into from
May 9, 2019

Conversation

alixdamman
Copy link
Collaborator

No description provided.

@alixdamman alixdamman requested a review from gdementen February 4, 2019 10:40
Copy link
Contributor

@gdementen gdementen left a comment

Choose a reason for hiding this comment

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

LGTM, except it needs a test.

@alixdamman
Copy link
Collaborator Author

A test already exists: see test_h5_io() in test_session.py

@gdementen
Copy link
Contributor

why didn't it pick up the failure then?

@alixdamman
Copy link
Collaborator Author

Maybe linked to the version used of pytables ?

@gdementen
Copy link
Contributor

I think it's more likely the version of pandas. But the first thing to do in any case when fixing a bug is reproduce it. Otherwise you don't know whether you actually fixed it or not. If that's indeed related to a lib version, we don't need to add any extra test but at least we could document it and possibly blacklist that particular version.

@gdementen
Copy link
Contributor

I guess the changelog message needs to be somehow changed.

@alixdamman alixdamman requested a review from gdementen May 8, 2019 10:39
Copy link
Contributor

@gdementen gdementen left a comment

Choose a reason for hiding this comment

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

LGTM

@alixdamman alixdamman merged commit 9264074 into larray-project:master May 9, 2019
@alixdamman alixdamman deleted the 702_fix_metadata branch May 9, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants