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

Cleanup codebase #9

Merged
merged 25 commits into from
Aug 24, 2023
Merged

Cleanup codebase #9

merged 25 commits into from
Aug 24, 2023

Conversation

adamjtaylor
Copy link
Collaborator

From the legacy htan-artist repo there are a number of half-developed or unused features in the codebase.

This PR removes these to allow our main branch to only include used and functional features.

Copy link

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

Nice! I'll let rixing do another review, but this looks great.

Copy link

@rxu17 rxu17 left a comment

Choose a reason for hiding this comment

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

I had a couple comments.

If any of the unused features plan to be included as part of this ticket, might be worthwhile moving them into a TODO README or leaving them there but adding TODO comments, thoughts?

main.nf Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

Similar comments as the above. They are referenced here.

@adamjtaylor
Copy link
Collaborator Author

adamjtaylor commented Aug 22, 2023

@rxu17 @tom Thanks for looking over. I've made a number of further changes.
Notably

  • Uses current release of Minerva (and avoids the need for two versions to be maintained)
  • Test datasets are tidied up with one example for H&E and one example for Multiplexed images.
  • For H&E which takes a fixed minerva story, this is added in the workflow rather than requiring a separate process to be launched just for a cp to occur
  • Processes are tagged with the ID which helps with debugging in Tower
  • All processes have labels for resource allocation (resource allocation is currently overkill so should optimise in a future PR)

I've been using these features (in the ajt_develop branch) for HTAN's release 4 and have had no obvious issues

@adamjtaylor adamjtaylor merged commit b9d5c21 into main Aug 24, 2023
3 checks passed
@adamjtaylor adamjtaylor deleted the cleanup branch August 24, 2023 14:52
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.

3 participants