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

chore(deps): allow xopen major version 2 #1532

Merged
merged 6 commits into from
Jul 11, 2024
Merged

chore(deps): allow xopen major version 2 #1532

merged 6 commits into from
Jul 11, 2024

Conversation

corneliusroemer
Copy link
Member

@corneliusroemer corneliusroemer commented Jul 11, 2024

There were no breaking changes of relevance to us, so we can allow xopen v2.

One small change: PipedCompressionReader/Writer was renamed to _pipedCompressionProgram.

If we want to support both v1 and v2, we have to do a bit of an ugly try/except import workaround. Probably worth keeping that for a few months because some programs people might want to use with augur potentially only support v1 at this point.

Once we merge and release this, we can install latest augur via conda-forge/bioconda natively on osx-arm64.

See xopen changelog: https://github.com/pycompression/xopen/blob/main/README.rst#v200-2024-03-26

Description of proposed changes

Related issue(s)

Checklist

  • Automated checks pass
  • Check if you need to add a changelog message
  • Check if you need to add tests
  • Check if you need to update docs

@corneliusroemer corneliusroemer marked this pull request as draft July 11, 2024 20:06
setup.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 69.89%. Comparing base (81badd2) to head (9064b55).
Report is 176 commits behind head on master.

Files with missing lines Patch % Lines
augur/io/file.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1532      +/-   ##
==========================================
- Coverage   69.90%   69.89%   -0.01%     
==========================================
  Files          75       75              
  Lines        7882     7887       +5     
  Branches     1933     1933              
==========================================
+ Hits         5510     5513       +3     
- Misses       2086     2088       +2     
  Partials      286      286              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

augur/io/file.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
See xopen changelog: https://github.com/pycompression/xopen/blob/main/README.rst#v200-2024-03-26

Maintain compatibility with v1 and v2, deprecate v1 with period of 4 months and requiring major version bump
setup.py Outdated Show resolved Hide resolved
DEPRECATED.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
augur/io/file.py Outdated Show resolved Hide resolved
Copy link
Member

@victorlin victorlin 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, just one more small thing

augur/io/file.py Outdated Show resolved Hide resolved
Co-authored-by: Victor Lin <13424970+victorlin@users.noreply.github.com>
@corneliusroemer
Copy link
Member Author

I removed the whitespace ;)

Would it be possible to release as 25.1.0? I know it's a bit silly given you released just recently - but it would mean we'd have full osx-arm64 conda support which would be rad :D

@corneliusroemer corneliusroemer merged commit 920cb9f into master Jul 11, 2024
21 of 22 checks passed
@corneliusroemer corneliusroemer deleted the xopen-v2 branch July 11, 2024 22:29
@victorlin
Copy link
Member

@corneliusroemer for sure! Let me take a look at #1534 and I can release both at the same time.

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