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

Fix ./bin/bump version.py on macOS #356

Merged
merged 7 commits into from
Mar 2, 2024

Conversation

AdamOrmondroyd
Copy link
Collaborator

Description

./bin/bump_version.py was not working on macOS, as its version of sed requires an backup for in-place editing, or an empty string to turn it off (-i'', without the space).

I tried to do something more subtle that would work cross-platform, but getting an empty string argument into subprocess.run() doesn't seem straightforward. I think a cleaner solution would be to automate this entirely in python or bash rather than mixing the two, but of course at the cost of a larger diff.

Fixes #355

Checklist:

  • I have performed a self-review of my own code
  • My code is PEP8 compliant (flake8 anesthetic tests)
  • My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
  • New and existing unit tests pass locally with my changes (python -m pytest)
  • I have added tests that prove my fix is effective or that my feature works
  • I have appropriately incremented the semantic version number in both README.rst and anesthetic/_version.py

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8132626) to head (3c82a11).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #356   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         2978      2978           
=========================================
  Hits          2978      2978           

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

@AdamOrmondroyd
Copy link
Collaborator Author

I think the macOS runners were having a bad time earlier, seems ok now

Copy link
Collaborator

@williamjameshandley williamjameshandley left a comment

Choose a reason for hiding this comment

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

OSX madness, but never mind!

Copy link
Collaborator

@williamjameshandley williamjameshandley left a comment

Choose a reason for hiding this comment

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

Apologies -- I didn't realise this was ready to go -- the red cross is misleading due to the failing but not essential checks.

@AdamOrmondroyd
Copy link
Collaborator Author

Apologies -- I didn't realise this was ready to go -- the red cross is misleading due to the failing but not essential checks.

the ❌ indeed do not spark joy. I kind of lost motivation with #359 since some of the warnings go away with recent development pandas

@AdamOrmondroyd AdamOrmondroyd merged commit e5b3bdf into handley-lab:master Mar 2, 2024
20 of 22 checks passed
@AdamOrmondroyd AdamOrmondroyd deleted the bump_version branch March 2, 2024 00:10
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.

bin/bump_version.py does not work on macOS
2 participants