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: correctly generate the MANIFEST when collating SampleData[MAGs] #352

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

misialq
Copy link
Collaborator

@misialq misialq commented Jan 8, 2025

This PR fixes a bug where the MANIFEST file in the SampleData[MAGs] artifact generated by the collate-sample-data-mags action would contain only entires from the last artifact which was being collated (the manifests were being overwritten). Instead, the entries will now be appended to the MANIFEST found in the first artifact.

@misialq misialq requested a review from Copilot January 8, 2025 09:03

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • q2_types/per_sample_sequences/tests/data/collated_mags/MANIFEST: Language not supported
  • q2_types/per_sample_sequences/tests/data/partitioned_mags/mag1/MANIFEST: Language not supported
  • q2_types/per_sample_sequences/tests/data/partitioned_mags/mag2/MANIFEST: Language not supported
Comments suppressed due to low confidence (1)

q2_types/per_sample_sequences/_methods.py:86

  • The comment has a grammatical error. It should be "it's" instead of "its".
# Since its present many times it will be overwritten, but that ok
@misialq
Copy link
Collaborator Author

misialq commented Jan 8, 2025

Hey @lizgehret, quick question about flake8: it complains about syntax but that's only because it's using python3.8 as a reference - this should be fine with 3.10. Should Python version also get bumped in the lint CI?

@lizgehret
Copy link
Member

Hey @lizgehret, quick question about flake8: it complains about syntax but that's only because it's using python3.8 as a reference - this should be fine with 3.10. Should Python version also get bumped in the lint CI?

Oh yeah you're totally right @misialq - I'll get that updated! Thanks for catching 🙂

@lizgehret
Copy link
Member

Okay @misialq lint has been fixed!

@misialq
Copy link
Collaborator Author

misialq commented Jan 9, 2025

Awesome, thanks @lizgehret! This is now ready for review by someone on your side of the pond 😁

@lizgehret lizgehret self-assigned this Jan 9, 2025
Copy link
Member

@lizgehret lizgehret left a comment

Choose a reason for hiding this comment

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

this looks reasonable to me, thanks @misialq!

@lizgehret lizgehret merged commit 11e14d8 into qiime2:dev Jan 9, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants