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

Deprecate setting default_file_mode, only allow setting to 'r' #1839

Merged
merged 2 commits into from Jun 18, 2021

Conversation

takluyver
Copy link
Member

This is step 2 of removing the config option added to ease the transition from 2.x to 3.x:

  1. Setting the default to anything other than 'r' (which is the default since 3.0) warns (done in Deprecate setting default_file_mode to values other than 'r' #1789 for h5py 3.2)
  2. Setting the default to 'r' warns, and to anything else is an error (this PR)
  3. Remove the default_file_mode option

We added the option (and indicated that the default would change) in 2.10 (released September 2019). The default changed to 'r' in 3.0 (October 2020). The warning for setting the default to modes other than 'r' was added in 3.2 (March 2021).

I'm provisionally marking this for 3.3, but I'm happy to leave it longer if people prefer. We could also start error-ing on modes other than r but leave the warning on setting the default to r for later.

@takluyver takluyver added this to the 3.3 milestone Mar 3, 2021
@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #1839 (95ce39f) into master (c616b0f) will decrease coverage by 1.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1839      +/-   ##
==========================================
- Coverage   88.40%   87.27%   -1.14%     
==========================================
  Files          17       17              
  Lines        2285     2184     -101     
==========================================
- Hits         2020     1906     -114     
- Misses        265      278      +13     
Impacted Files Coverage Δ
h5py/_hl/files.py 91.70% <100.00%> (+0.30%) ⬆️
h5py/_hl/filters.py 84.97% <0.00%> (-7.81%) ⬇️
h5py/_hl/attrs.py 95.20% <0.00%> (-0.99%) ⬇️
h5py/_hl/dims.py 95.18% <0.00%> (-0.70%) ⬇️
h5py/_hl/datatype.py 95.00% <0.00%> (-0.66%) ⬇️
h5py/_hl/selections.py 87.43% <0.00%> (-0.57%) ⬇️
h5py/_hl/base.py 95.37% <0.00%> (-0.27%) ⬇️
h5py/_hl/dataset.py 92.30% <0.00%> (-0.22%) ⬇️
h5py/_hl/group.py 96.62% <0.00%> (-0.13%) ⬇️
h5py/_hl/vds.py 97.33% <0.00%> (-0.04%) ⬇️
... and 1 more

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 c616b0f...95ce39f. Read the comment docs.

@takluyver
Copy link
Member Author

I'm aiming for 3.3 fairly soon. On the one hand, 3 months is fairly short for going from a warning to an error. On the other hand, the proper fix - adding an explicit mode to h5py.File calls wherever necessary - is simple and obvious, and people have had the best part of 2 years to make that change (since 2.10 warned that the default was changing). So I will merge this in a few more days if no-one objects.

@tacaswell
Copy link
Member

Given @aragilar' comment is #1909 (comment) I'm going to merge this.

@tacaswell tacaswell merged commit c731c3d into h5py:master Jun 18, 2021
@takluyver takluyver deleted the deprecate-default-file-mode-2 branch November 2, 2021 11:57
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.

None yet

2 participants