-
Notifications
You must be signed in to change notification settings - Fork 6
python 3.10, 3.11, 3.12 support | model #51
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
Conversation
sidmohan0
commented
Aug 5, 2024
- Passing tests using tox for Python 3.10, 3.11, and 3.12
- Simplified requirements, pinned versions only for Spacy, Pydantic, and Requests
- switched to different PII detection model, en_core_web_lg from en_spacy_pii_fast -> allowed us to use newer Spacy
FYI, when i tried to pinn Requests >= 2.82.0 i got this error during build |
I ended up making a few changes to the cicd files, notably the addition of the 'Free Disk Space (Ubuntu)' step to address issues around running out of disk space |
import subprocess | ||
|
||
subprocess.run( | ||
["python", "-m", "spacy", "download", "en_core_web_lg"], check=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should figure out if python or python3 needs to be called here. I know this won't work on my system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be able to find the binary running this file and using that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the usage I've seen referenced and also used online has been python, not python3. is it just a matter of adding handling here for python vs python3?
I ended up changing this command to download the binary directly, not sure if it directly addresses the first concern though..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This calls out to the OS. If python doesn't exist this will fail.
I'd suggest this:
import sys
interpreter_location = sys.executable
This statement is helpful:
On all platforms, passing sys.executable is the recommended way to launch the current Python interpreter again, and use the -m command-line format to launch an installed module.
https://docs.python.org/3/library/subprocess.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks - just implemented that in 924b47b
and all checks have passed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!