chore(medcat-trainer): update deps #325
Conversation
… upgrade django, django-polymorphic
tomolopolis
changed the title
| #!/bin/sh | ||
|
|
||
| # Use venv where uv installed dependencies (explicit path) | ||
| PY=/home/.venv/bin/python |
There was a problem hiding this comment.
As I've not seen this full path pattern before:
Could you instead source the venv and run normal python commands? Alternatively can you add it to the path in the dockerfile? ENV PATH="/app/.venv/bin:$PATH" I think would solve it?
3rd option would be uv run python .. ?
Generally just it seems odd having to explicit path everything. If explicit path is preferred then all good, I see the pros/cons there
mart-r
left a comment
mart-r
left a comment
There was a problem hiding this comment.
I've got a few commends on the explicit usage of the virtual environment path over using uv run python or uv run uwsgi. Mostly a preference thing, but what I suggested seems to be what uv is designed for (and is already used in some parts of this change anyway).
And then there's another comment regarding the workflow using python 3.11 whereas the container using 3.12. Perhaps using the same one would make a bit more sense?
Approved anyway - the above (especially uv run stuff) are somewhat minor.
| @@ -0,0 +1,26 @@ | |||
| [project] | |||
| name = "medcattrainer-webapp" | |||
| version = "1.0.0" | |||
There was a problem hiding this comment.
Perhaps having this sync up would be useful?
| #!/bin/sh | ||
|
|
||
| # Use venv where uv installed dependencies (explicit path) | ||
| PY=/home/.venv/bin/python |
There was a problem hiding this comment.
Could we use uv run python here instead? To me it feels cleaner than referring to the spcific path of the virtual environment. This should work in subfolders as well (i.e uv should find the env from parent folder).
medcat-trainer/webapp/Dockerfile
Outdated
| && uv sync --frozen --no-install-project | ||
|
|
||
| # Ensure venv has pip (uv venvs don't include it; spacy download needs it) | ||
| RUN .venv/bin/python -m ensurepip --upgrade |
There was a problem hiding this comment.
Again, I feel like uv run python -m ensurepipe --upgrade would look cleaner (or more uv-like)
medcat-trainer/webapp/Dockerfile
Outdated
| # Download spaCy models (only requires spaCy, not application code) | ||
| ARG SPACY_MODELS="en_core_web_md" | ||
| RUN for SPACY_MODEL in ${SPACY_MODELS}; do python -m spacy download ${SPACY_MODEL}; done | ||
| RUN for SPACY_MODEL in ${SPACY_MODELS}; do .venv/bin/python -m spacy download ${SPACY_MODEL}; done |
There was a problem hiding this comment.
Same here regaring .venv/bin/python - I think uv run python sounds better.
3 participants
uvdjangodjango-polymorphic