-
Notifications
You must be signed in to change notification settings - Fork 41
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
ENH: Migrated partition and collating actions from q2-moshpit
to q2-types
#334
ENH: Migrated partition and collating actions from q2-moshpit
to q2-types
#334
Conversation
Hello @lizgehret, Thank you! The error is this:
|
Hey @VinzentRisch, Happy to help! I'll take a look at this shortly. |
Hey @VinzentRisch, Apologies for the delay on this! I've been playing around with this locally, and this is definitely a tricky one. I'm going to try a few different things to work around these circular imports, but this may be something we want to discuss in our joint lab meeting next month. I suspect this will be something that may come up more as we continue to add types/formats from the metagenomic ecosystem. EDIT: One additional thing I'm thinking about is that all of the additions in this PR are methods, rather than strictly being types/formats/transformers. I'm wondering if these are things we even want to be moving into q2-types - but I'll discuss with other folks on the team about this and circle back! |
I'm going to give this a shot as well. q2-types has caused import issues for basically everyone at this point. I think a refactor may be in order. |
Once #342 is merged, it should be relatively straight-forward to rebase this PR ontop of it. The registration will end up in |
Hey @VinzentRisch, We should be all set to continue working on this one. I've fixed the merge conflicts and we no longer end up with import cycles. Of note, I did delete one method in 08e3fd1 which operated on a format that was removed in #338 We might consider moving the plugin method registrations into the smaller
|
Hi @ebolyen |
Thanks for all of the work on this @VinzentRisch! @misialq, any chance you could review this one? @ebolyen has already reviewed the structure and said that it all looks good, so it's just the functionality at this point that needs review and we think you might have the most context there. If you can't get to it, I can review this one but it'll be a few weeks as I'm about to head out to teach a workshop next week and then am tied up the week after that - so we're looking at the 2nd week of September before I can get to it. |
Hey @gregcaporaso, sure thing - I'll give it a go this week! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey all, this looks good to me too - thanks! 🚀
closes #332
partition_feature_data_mags
,collate_feature_data_mags
,collate_ortholog_annotations
,collate_orthologs
,partition_orthologs
,partition_sample_data_mags
,collate_sample_data_mags
,_validate_mag_ids
and_validate_num_partitions
from q2-moshpit to q2-types.collate_orthologs
andpartition_orthologs
.collate_annotations
tocollate_ortholog_annotations
.shutil.move
toduplicate
incollate_orthologs
action to be consistent with other collate actions.Run it locally
Download test files
PR-334-types.zip
Test it out!