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

Add scanpy #25

Merged
merged 9 commits into from
Aug 26, 2024
Merged

Add scanpy #25

merged 9 commits into from
Aug 26, 2024

Conversation

Sanaz01
Copy link
Collaborator

@Sanaz01 Sanaz01 commented Apr 30, 2024

Description

Related Issue

Example

scanpy/Dockerfile Outdated Show resolved Hide resolved
scvi-tools/Dockerfile Outdated Show resolved Hide resolved
scvi-tools/Dockerfile_old Outdated Show resolved Hide resolved
@tefirman tefirman self-requested a review August 23, 2024 21:00
Copy link
Member

@tefirman tefirman left a comment

Choose a reason for hiding this comment

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

First off, holy crap, really sorry it took me so long to re-review... 🤦‍♂️ Those updates to pip look awesome, thanks for updating those. One last thing to update though, just for WILDS-style purposes, mind adding the tag to the filenames? (see comments for details).

scanpy/Dockerfile Outdated Show resolved Hide resolved
scvi-tools/Dockerfile Outdated Show resolved Hide resolved
@Sanaz01
Copy link
Collaborator Author

Sanaz01 commented Aug 24, 2024

No worries at all. I have changed the Dockerfile names and updated package version as well!

@tefirman tefirman self-requested a review August 24, 2024 15:52
Copy link
Member

@tefirman tefirman left a comment

Choose a reason for hiding this comment

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

Interesting update! Yesterday, I noticed that the linter GitHub Action wasn't performing its check, turns out we've recently added some security features in WILDS to restrict which action types are allowed. Just needed to add the hadolint action to the list of acceptable actions and it ran as expected on the next push.

That being said, you'll now notice that the check failed because of a few small optimization issues with scvi-tools (follow link or see screenshot below). Mind fixing these as well? Happy to chat Monday to go through these as well!

image

@Sanaz01
Copy link
Collaborator Author

Sanaz01 commented Aug 25, 2024

I have made the following changes to scvi-tools: cbaf540

  1. add --no-cache-dir
  2. added version to packages
  3. merged multiple RUN instructions

Copy link
Member

@tefirman tefirman left a comment

Choose a reason for hiding this comment

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

Found one last issue. I tried building both images locally and the scvi-tools image errored out. Looks like combining the two pip calls threw things off because of the --index-url argument. I just split it into two pip calls again, but on the same RUN command, compiled fine from there. Thanks for your hard work and patience on this Sanaz! 🥳

@tefirman tefirman merged commit 897de13 into getwilds:main Aug 26, 2024
2 checks passed
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.

2 participants