Skip to content

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

Merged
merged 24 commits into from
Aug 6, 2024
Merged

Conversation

sidmohan0
Copy link
Contributor

  • 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

@sidmohan0 sidmohan0 requested review from pselvana and sroy9675 August 5, 2024 23:36
@sidmohan0
Copy link
Contributor Author

FYI, when i tried to pinn Requests >= 2.82.0 i got this error during build
ERROR: Could not find a version that satisfies the requirement Requests>=2.82.0 (from versions: 0.2.0, 0.2.1, 0.2.2, 0.2.3, 0.2.4, 0.3.0, 0.3.1, 0.3.2, 0.3.3, 0.3.4, 0.4.0, 0.4.1, 0.5.0, 0.5.1, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.7.0, 0.7.1, 0.7.2, 0.7.3, 0.7.4, 0.7.5, 0.7.6, 0.8.0, 0.8.1, 0.8.2, 0.8.3, 0.8.4, 0.8.5, 0.8.6, 0.8.7, 0.8.8, 0.8.9, 0.9.0, 0.9.1, 0.9.2, 0.9.3, 0.10.0, 0.10.1, 0.10.2, 0.10.3, 0.10.4, 0.10.6, 0.10.7, 0.10.8, 0.11.1, 0.11.2, 0.12.0, 0.12.1, 0.13.0, 0.13.1, 0.13.2, 0.13.3, 0.13.4, 0.13.5, 0.13.6, 0.13.7, 0.13.8, 0.13.9, 0.14.0, 0.14.1, 0.14.2, 1.0.0, 1.0.1, 1.0.2, 1.0.3, 1.0.4, 1.1.0, 1.2.0, 1.2.1, 1.2.2, 1.2.3, 2.0.0, 2.0.1, 2.1.0, 2.2.0, 2.2.1, 2.3.0, 2.4.0, 2.4.1, 2.4.2, 2.4.3, 2.5.0, 2.5.1, 2.5.2, 2.5.3, 2.6.0, 2.6.1, 2.6.2, 2.7.0, 2.8.0, 2.8.1, 2.9.0, 2.9.1, 2.9.2, 2.10.0, 2.11.0, 2.11.1, 2.12.0, 2.12.1, 2.12.2, 2.12.3, 2.12.4, 2.12.5, 2.13.0, 2.14.0, 2.14.1, 2.14.2, 2.15.1, 2.16.0, 2.16.1, 2.16.2, 2.16.3, 2.16.4, 2.16.5, 2.17.0, 2.17.1, 2.17.2, 2.17.3, 2.18.0, 2.18.1, 2.18.2, 2.18.3, 2.18.4, 2.19.0, 2.19.1, 2.20.0, 2.20.1, 2.21.0, 2.22.0, 2.23.0, 2.24.0, 2.25.0, 2.25.1, 2.26.0, 2.27.0, 2.27.1, 2.28.0, 2.28.1, 2.28.2, 2.29.0, 2.30.0, 2.31.0, 2.32.2, 2.32.3)
ERROR: No matching distribution found for Requests>=2.82.0

@sidmohan0
Copy link
Contributor Author

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

@sidmohan0 sidmohan0 removed the request for review from sroy9675 August 6, 2024 00:53
import subprocess

subprocess.run(
["python", "-m", "spacy", "download", "en_core_web_lg"], check=True
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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..

Copy link
Contributor

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

Copy link
Contributor Author

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!

@sidmohan0 sidmohan0 requested a review from pselvana August 6, 2024 16:51
Copy link
Contributor

@pselvana pselvana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sidmohan0 sidmohan0 merged commit 492ab5c into dev Aug 6, 2024
7 checks passed
@sidmohan0 sidmohan0 deleted the feature/py312-support branch August 6, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants