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 study/sample metadata updates not updating permissions #318

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

kjsanger
Copy link
Member

@kjsanger kjsanger commented Jun 3, 2024

The enhance-secondary-metadata was omitting permission updates when updating metadata. It was also almost duplicating functions already present in update-secondary-metadata.

Remove the enhance-secondary-metadata script.

Add a clause to the update-secondary-metadata case statement to capture the cases covered by enhance_secondary_metadata.

Refactor to remove duplicate code.

Update fixtures and tests with more descriptive names.

@kjsanger kjsanger added the bug Something isn't working label Jun 3, 2024
@mgcam mgcam self-requested a review June 6, 2024 09:38
@mgcam
Copy link
Member

mgcam commented Jun 6, 2024

The documentation in npg_irods.cli.update_secondary_metadata states that the code can be used for Illumina and ONT data, while we are extending the use to any arbitrary platform. Could the documentation be extended please.

The enhance-secondary-metadata was omitting permission updates when
updating metadata. It was also almost duplicating functions already
present in update-secondary-metadata.

Remove the enhance-secondary-metadata script.

Add a clause to the update-secondary-metadata case statement to
capture the cases covered by enhance_secondary_metadata.

Refactor to remove duplicate code.

Update fixtures and tests with more descriptive names.
@kjsanger kjsanger force-pushed the bug/metadata-enhance-perms branch from 6d71895 to 79734eb Compare June 6, 2024 13:25
Copy link
Member

@mgcam mgcam left a comment

Choose a reason for hiding this comment

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

It's good to drop code repetitions. Please see a suggestion for a minor change.

@nerdstrike has run the code for Elembio data - thank you, @nerdstrike

@@ -46,6 +52,7 @@ class Platform(Enum):
"""An analysis platform (instrument or analysis technology), named by manufacturing
company."""

OTHER = 0
Copy link
Member

Choose a reason for hiding this comment

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

For completeness, Elembio platform should be added.

Update the iRODS path for Ultima Genomics from "ug" to "ultimagen"
@mgcam mgcam merged commit 115f53d into wtsi-npg:devel Jun 6, 2024
5 checks passed
@kjsanger kjsanger deleted the bug/metadata-enhance-perms branch October 15, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants