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 writing starfile #80

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Conversation

hanjinliu
Copy link
Collaborator

Fixes #79

The reason was the usage of f.writelines(...).

  • I added the to_string method to StarWriter, as always using the same method to make string is easier to maintain and generally safer.
  • I modified the test_parse_na test to use tmpdir built-in fixture because when I pytested locally using temporary file caused permission error.

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.33%. Comparing base (5b19406) to head (9865fbd).
Report is 24 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   80.93%   82.33%   +1.39%     
==========================================
  Files           7        7              
  Lines         278      300      +22     
==========================================
+ Hits          225      247      +22     
  Misses         53       53              

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

@alisterburt
Copy link
Collaborator

Thanks for the fix @hanjinliu ! Will take a proper look and likely merge tomorrow :-)

@alisterburt alisterburt merged commit 775e1c6 into teamtomo:main Jan 8, 2025
5 checks passed
@alisterburt
Copy link
Collaborator

@hanjinliu I've merged here, thanks for taking the time to make the PR!

I've given you maintainer access to the repo, this gives you the ability to fix/cut releases yourself if similar small issues come up. Once your local repo is set up pushing a release should be as easy as pushing a tagged commit to main with the "v*" pattern, triggering the deployment workflow here

if: success() && startsWith(github.ref, 'refs/tags/') && github.event_name != 'schedule'

git tag -a v0.5.11 -m "v0.5.11"
git push --follow-tags upstream/main

If you'd rather not push the release yourself let me know and I'll do it, just trying to distribute maintenance burden a little :-)

@hanjinliu hanjinliu deleted the fix-additional-lines branch January 9, 2025 01:13
@hanjinliu
Copy link
Collaborator Author

Thank you for inviting me to the collaboration, @alisterburt ! I think I understand how to make releases now.

@alisterburt
Copy link
Collaborator

@hanjinliu great to have you on board! I haven't seen a tagged release pushed yet, let me know if something is unclear/you need some more direction 🙂

@hanjinliu
Copy link
Collaborator Author

Hi @alisterburt , sorry for being late. I think 0.5.11 is correctly released now.

@alisterburt
Copy link
Collaborator

Thanks for pushing the release! 🙂

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.

Unnecessary lines with starfile=0.5.10
3 participants