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

maintenance: h5py.default_file_mode is deprecated #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sjanssen2
Copy link

When starting Qiita, the log reports about a deprecated use of h5py.get_config().default_file_mode = 'r'. This function was indeed deprecated with version 3.0.0. I therefore use the packaging lib to check if the used h5py version is older than that before trying to manually set to 'r'.

Should you feel we anyways would not use older h5py versions, we can delete these lines 15-20 in qiita_files/util.py altogether

@antgonza
Copy link
Member

Thank you @sjanssen2. Do you think is worth supporting older versions of h5py or just support the latests? I vote to just support the latests - thus, not needing packaging and checking.

@sjanssen2
Copy link
Author

Hi @antgonza I tend to agree that we don't have to support older versions, however I don't feel to competent to consider all edge cases. Thus, I chose the more conservative method. Your choice :-) But I'd be happy to get rid of this test altogether.

@antgonza
Copy link
Member

In my experience, is better to just rip off the bandaid and then deal with any tentative issues if/when they arise. So, please remove that extra conditional and the need of packaging. Thanks.

@antgonza
Copy link
Member

Just to confirm before merging, what version of h5py should we be using and are there any other updated dependencies for that?

@antgonza
Copy link
Member

antgonza commented Apr 1, 2024

Also, I just realized that the actions are not testing against python 2.7 and while the plan is to stop supporting ASAP we still have at least 1 plugin that uses it.

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