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 user control for whether models are downloaded automatically #135

Open
vrkosk opened this issue Mar 22, 2024 · 2 comments
Open

Add user control for whether models are downloaded automatically #135

vrkosk opened this issue Mar 22, 2024 · 2 comments

Comments

@vrkosk
Copy link

vrkosk commented Mar 22, 2024

Currently, when MS2PIPFeatureGenerator calls ms2pip.correlate(), the latter may download xgboost_models_files from the Internet if they are not present in the model_dir directory. Can we please have a control to disable this behaviour?

It's possible to do by monkey patching. At the top of your script, before importing anything from MS2Rescore or MS2PIP, add these lines:

import urllib.request

def _raise_urllib_exception(*args, **kwargs):
    raise urllib.error.URLError("Downloading from the Internet is prevented in this build.")

urllib.request.urlretrieve = _raise_urllib_exception

from ms2rescore.feature_generators.ms2pip import MS2PIPFeatureGenerator

The code targeted is in ms2pip/_utils/xgb_models.py, def _download_model(), which uses urllib.request.urlretrieve. The trick relies on the fact that there is always only one instance of an imported module, so messing with urllib.request before ms2pip imports it guarantees that ms2pip sees the same monkey-patched instance.

Obviously, this is rather poor practice and some option passed to MS2PIPFeatureGenerator would be much nicer.

@RalfG
Copy link
Member

RalfG commented Mar 28, 2024

Hi @vrkosk,

Thanks for the suggestion! We will look into adding this in MS²PIP, so a monkey patch is not required and it simply is provided as an option.

If auto-download is disabled, and the required model is missing from the directory, you would expect an exception to be raised?

Best,
Ralf

@vrkosk
Copy link
Author

vrkosk commented Mar 28, 2024

Thank you for considering it! Any way to signal the error is fine as long as it's in the MS2PIPFeatureGenerator documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants