Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
GeorgWa
left a comment
GeorgWa
left a comment
There was a problem hiding this comment.
Where is the code change where we actually stop downloading models at import time?
Shouldn't this be somewehere?
|
|
||
| To deprecate a variable: | ||
| > import sys, ModuleWithDeprecations | ||
| > sys.modules[__name__].__class__ = ModuleWithDeprecations |
There was a problem hiding this comment.
This example confuses me a lot.
Does it need to be sys or is this just and example?
What is modules[__name__]?
Can you provide a simpler example?
There was a problem hiding this comment.
this is literally what needs to be done: https://github.com/MannLabs/alphapeptdeep/pull/229/files#diff-c01c08ac727782806e8ddbb06d6e9b41f98c7505b81de9366a03581826c264a6R50
There was a problem hiding this comment.
maybe this deprecation mechanism is also too much?
| sys.modules[__name__].__class__ = ModuleWithDeprecations | ||
|
|
||
| LOCAL_MODAL_ZIP_NAME = global_settings["local_model_zip_name"] | ||
| MODEL_URL = global_settings["model_url"] |
There was a problem hiding this comment.
Should this be an all caps constant if it's actually loaded from a config?
Why not using global_settings["model_url"] directly?
There was a problem hiding this comment.
well, it's a string and somehow constant (within the lifetime of a peptdeep instance) .. that's why I chose ALL_CAPS
| if not os.path.exists(model_zip): | ||
| download_models() |
There was a problem hiding this comment.
this is where the downloading on import time was done ..
.. and this is where it went: https://github.com/MannLabs/alphapeptdeep/pull/229/files#diff-c01c08ac727782806e8ddbb06d6e9b41f98c7505b81de9366a03581826c264a6R50
@GeorgWa
2 participants
Avoid the toplevel code that downloaded the module already if just
alphadia -vwas done ..Also, added a potentially reusable mechanism for deprecating module-level variables