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

Expose READ/WRITE gzip consts (in addition to existing FTEXT, FEXTRA, etc) #115

Merged
merged 3 commits into from Jul 15, 2022
Merged

Expose READ/WRITE gzip consts (in addition to existing FTEXT, FEXTRA, etc) #115

merged 3 commits into from Jul 15, 2022

Conversation

alexander-beedie
Copy link

@alexander-beedie alexander-beedie commented Apr 12, 2022

Added/exposed the only missing top-level gzip consts, READ and WRITE, with the expected values. Improves drop-in completeness / replacement behaviour when substituting gzip imports with igzip imports.

(Note: all of the other public consts are already exposed/available: FCOMMENT, FEXTRA, FHCRC, FNAME, FTEXT).

Added test coverage to confirm the presence (and expected value) of all of these.

@rhpvorderman
Copy link
Collaborator

Looks good! Thanks! I will merge once the tests have succeeded. Can you fix the small linting error? I try to keep the library PEP8 compliant.

This should end up in python-isal 1.0. I am currently waiting for ISA-L to update to 2.31 before I release that version.

@codecov
Copy link

codecov bot commented Apr 12, 2022

Codecov Report

Merging #115 (66385e2) into develop (396c1e7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #115   +/-   ##
========================================
  Coverage    98.03%   98.03%           
========================================
  Files            2        2           
  Lines          254      255    +1     
  Branches        66       66           
========================================
+ Hits           249      250    +1     
  Misses           2        2           
  Partials         3        3           
Impacted Files Coverage Δ
src/isal/igzip.py 97.98% <100.00%> (+<0.01%) ⬆️

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 396c1e7...66385e2. Read the comment docs.

@alexander-beedie
Copy link
Author

Looks good! Thanks! I will merge once the tests have succeeded. Can you fix the small linting error? I try to keep the library PEP8 compliant.

No problem; committed the spacing fix ;)

@rhpvorderman
Copy link
Collaborator

Whoops, and then I forgot about this PR. Sorry not to merge this earlier.
I don't know when I can release. I have updated this library quite a bit to depend on bug fixes and build improvements in ISA-L 2.31 under the assumption that the ISA-L release was imminent. Unfortunately this has been the longest gap between releases for ISA-L yet.

@rhpvorderman rhpvorderman merged commit 523f45b into pycompression:develop Jul 15, 2022
@alexander-beedie
Copy link
Author

Whoops, and then I forgot about this PR. Sorry not to merge this earlier.

No problem; it's a simple enough patch to keep in sync locally until it makes it out into the wild - thanks! ;)

@rhpvorderman
Copy link
Collaborator

It made it into the wild. I just released now, wheels should be available shortly. Thanks again!

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.

None yet

2 participants