feat(medcat): CU-869azeyvz Add scripts download CLI #206
Conversation
|
Task linked: CU-869azeyvz Add script downloading enpoint to lib |
alhendrickson
left a comment
alhendrickson
left a comment
There was a problem hiding this comment.
Hey this code looks good to me.
Definitely I've got wider thoughts about the packaging process in general, but ultimately the python CLI looks like a good plan
medcat-v2/medcat/__main__.py
Outdated
|
|
||
| def main(*args: str): | ||
| if not args: | ||
| print("Usage: python -m medcat download-scripts [DEST] [log_level]", |
There was a problem hiding this comment.
Minor - would make a const for these usage statements, so it's the same as below
| with open(target, "wb") as f: | ||
| f.write(zf.read(m)) | ||
|
|
||
| logger.info("Scripts extracted to: %s", dest / SCRIPTS_PATH) |
There was a problem hiding this comment.
Minor & pure opinion - can we make dest the whole string? Eg default to "./medcat-scripts", but if I provide destination then just use it without forcing SCRIPTS_PATH on me. EG git clone url.git my-folder would put it all in my-folder, not a subfolder in there
To clarify I'd be looking for this behavior
python -m medcat download-scripts /some/file/path
ls /some/file/path
<pure contents of the zip>
# Compared to the current, where ls /some/file/path would always contain a folder "medcat-scripts"
There was a problem hiding this comment.
Yeah, I did look at that as well and go "do we need the extra implicit folder?".
|
|
||
| def _get_medcat_version() -> str: | ||
| """Return the installed MedCAT version as 'major.minor'.""" | ||
| version = importlib.metadata.version("medcat") |
There was a problem hiding this comment.
Not important - what happens if this is run from the main branch?
There was a problem hiding this comment.
Yeah I'm thinking in future an optional arg that lets me specify a tag would be really useful - eg if I want to run this from my main branch for testing or whatever. Prediction is having this not always force the version lookup will be needed one day.
There was a problem hiding this comment.
Currently, if you're running it off main branch, it'll have the version specified in pyproject.toml. It's at 2.0.0 currently, but we can bump it to 2.2.0 so it find at least something.
Though the other option would be to look for the latest tag in the history if we've got a local build instead.
EDIT: That would only work for editable installs - you normally don't have git history available.
There was a problem hiding this comment.
Thats cool - I dont see the suggested arg being useful anytime soon then if it all runs fine, so all good to merge
Instead, add everything directly to the folder specified
This PR adds a command line interface to download
medcat-scripts.The way it works is that it:
medcat-scripts/v2.2.0formedcat==2.2.0)The CLI can be used as follows:
Where both arguments are optional.
If
[DEST]is not specified, the current working directory us used. The contents ofmedcat-scriptsfolder will be copied into this destination.By specifying
log_levelyou can add or remove the logging level (defaults to INFO).The idea is that this can be used instead of cloning
working_with_cogstack(which is what was used in v1). But with the additional safeguards of getting the correct version of the scripts that are (somewhat) guaranteed to work with your current installation.