Skip to content
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

Update findPythonDeps #4682

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Oct 15, 2024

I noticed that the script failed to detect e.g. jupyter-core because the package_name entry is jupyter_core. However that isn't consistent:

  • "PyQt5-sip": 'key': 'pyqt5-sip', 'package_name': 'PyQt5-sip'
  • "jupyter-core": 'key': 'jupyter-core', 'package_name': 'jupyter_core'

I fixed that and used the opportunity to generally do some updates. In total:

  • Require Python 3.3 3.6 (due to f-strings and subprocess.DEVNULL [3.3])
  • Add/update documentation
  • Use f-strings for formatted output
  • Fix detecting packages where the name might appear as the "key" not the "package_name"
  • Set PYTHONNOUSERSITE
  • Use venv module instead of virtualenv to require less packages

This should make the script more robust and easier to understand and maintain. Especially the description at the top should be very helpful for those that don't know it yet.

I used PyLint to check the script. In one place there was an unused argument that should have been used but wasn't. Putting the "main" code into a function allowed to detect that as there are no more global variables.

@Flamefire Flamefire force-pushed the 20241015105526_new_pr_VNhxgSMfnR branch 3 times, most recently from 0f5d9fa to d81cb35 Compare October 15, 2024 09:07
- Require Python 3.3
- Add/update documentation
- Use f-strings for formatted output
- Fix detecting packages where the name might appear as the "key" not the "package_name"
- Set PYTHONNOUSERSITE
- Use venv module instead of virtualenv to require less packages
@Flamefire Flamefire force-pushed the 20241015105526_new_pr_VNhxgSMfnR branch from d81cb35 to b01a52b Compare October 15, 2024 09:11
@Micket
Copy link
Contributor

Micket commented Oct 15, 2024

Require Python 3.3

Typo (given that you use f-strings, so it would be python3.6 at least)?

@Flamefire Flamefire force-pushed the 20241015105526_new_pr_VNhxgSMfnR branch 2 times, most recently from 92b7477 to b01a52b Compare October 15, 2024 12:06
@Flamefire
Copy link
Contributor Author

Typo (given that you use f-strings, so it would be python3.6 at least)?

Yes, wasn't aware that f-strings were so late. Also the change from virtualenv to venv wasn't complete.

The script will (also) be run with a loaded Python module. So after this change it can no longer be used to find packages for Python 2 or < 3.6. That is 2018b for Python2 and 2016b for Python 3.5. I'd say this is fine as there is likely no one adding ECs for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants