-
Notifications
You must be signed in to change notification settings - Fork 106
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
Allow assignment cache to be saved to and loaded from datadir #444
Allow assignment cache to be saved to and loaded from datadir #444
Conversation
pvanheus
commented
May 5, 2022
- Support download assignment cache to datadir and using from datadir
- Switch to using pip for all installs (thanks for tip from Wolfgang Maier)
- Change logic for finding constellations files to use latest version
* Switch to using pip for all installs (thanks for tip from Wolfgang Maier) * Change logic for finding constellations files to use latest version
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.
Very cool work @pvanheus !
One general remark: I know you didn't touch this in this PR, but I'm somewhat unhappy with the earlier decision to accept a --datadir only if the pango data in it is newer than the package-level one. I think if a user specifies --datadir, they are fully responsible for its contents. If they want to use an older version of data, let them do so. In fact, for Galaxy the current version check creates some extra effort because with data managers we want to be able to enforce use of an old data version for reproducibility of prior jobs. Right now we're cheating by prepending the datadir version with a "v" to make it appear more recent to the check: With the changes in this PR, it could also make sense to use whatever data components are found in --datadir even if there is no pangolin-data in it. E.g. a user might want to use only a datadir constellations, but go with the installed versions of everything else. |
Thanks so much for this @pvanheus! I agree with @wm75's comments (yikes about prepending the "v"). I think it would be fine to simply accept what the user provides in datadir; also fine to warn the user if the datadir version is older than the installed version; but heavy-handed to ignore datadir if the version is equal to or less than the installed version, and not OK to ignore without warning the user, as I think is done for constellations unless I'm missing something. |
I also agree, I can see cases where people would want to use historical data. My only concern is that there isn't always going to be backwards compatibility, but I guess if it finds the correct files then it'll work! |
I have added a flag ( In terms of backwards compatibility, can a |
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.
Thanks so much for the changes, @pvanheus, this is looking much cleaner now!
I have one more question and a couple nitpicky requests, but it's looking really good now with the more uniform version-finding and pip usage.
…ddress PR comments
This looks terrific now, thanks so much Peter for all of your work on this and Wolfgang for the helpful tips! |
Thanks so much @pvanheus, @wm75 and @AngieHinrichs! I think this will be really useful for pangolin users. 😁 |