ersilia-os / ersilia

The Ersilia Model Hub, a repository of AI/ML models for infectious and neglected disease research.
https://ersilia.io
GNU General Public License v3.0
224 stars 147 forks source link

🐈 Task: Remove duplicate flag --repo_path from the fetch command #1370

Closed DhanshreeA closed 2 hours ago

DhanshreeA commented 2 weeks ago

Summary

Refer to #1262

The --repo_path flag is a duplicate of the --from_dir flag since both expect the local path of a model's directory (repository). Currently the --from_dir flag is used only in setting the model source, and is not really used elsewhere. Users and contributors mainly use the repo_path flag to fetch a locally available model, however for the sake of consistency across nomenclature, we should favor keeping --from_dir instead of --repo_path, since that is how the other source flags are specified (eg --from_github, --from_dockerhub etc)

Objective(s)

Documentation

No response

snufkinwa commented 1 week ago

Hi, I'd like to contribute to this issue if it's still available.

DhanshreeA commented 1 week ago

Hi @snufkinwa thank you for your interest in working on this. We are still finalizing whether we want to address this issue or not.

DhanshreeA commented 1 week ago

@miquelduranfrigola @GemmaTuron what do you both think? I know we've been using repo_path from quite some time now but it was also developed in the early days of ersilia when we hadn't formalized the CLI to the extent that exists now. So I vote removing this flag and actually using the from_dir flag.

snufkinwa commented 1 week ago

@DhanshreeA Let me know! If there are any other first-time issues I can contribute to, I'd love to help.

DhanshreeA commented 1 week ago

@snufkinwa I have discussed with @miquelduranfrigola - we do indeed want to get rid of this. If you're still interested in taking this up, I'll be happy to provide support.

miquelduranfrigola commented 1 week ago

Thanks @DhanshreeA

Completely agree with this. We just need to be extra careful with the pipelines/workflows that are using --repo_path

snufkinwa commented 1 week ago

@DhanshreeA Yes, I'll take this on—thank you for the support! I’m on board with removing --repo_path. Just to make sure I’m covering everything, could you let me know which specific pipelines or workflows rely on --repo_path? I want to make sure the transition goes smoothly.

--repo_path is in 5 files, the file with Model Fetcher constructor and --from_dir is in ersilia/cli/commands/fetch.py So the changes that need to be made is in this file. Making sure I have clarification for

We just need to be extra careful with the pipelines/workflows that are using --repo_path

DhanshreeA commented 1 week ago

Hi @snufkinwa so on a quick review of where --repo_path is used across all the repositories within Ersilia, I see that it is mainly used in our model test workflows. For now I think it's best if you just focus on the fetch.py file (as you've linked above).

DhanshreeA commented 2 hours ago

This has been addressed in the linked PR.