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

feat(python): use findpython #10097

Merged
merged 2 commits into from
Feb 2, 2025
Merged

feat(python): use findpython #10097

merged 2 commits into from
Feb 2, 2025

Conversation

abn
Copy link
Member

@abn abn commented Jan 23, 2025

With this change Poetry will use findpython for Python version discovery. This helps remove Poetry's own custom "best effort" Python version discovery and hopefully also resolve a lot of issues related to the discovery of available Python versions in a given environment.

This change largely focuses on the Python Manager and not the Environment Manager. The latter could also be improved once this has landed.

Testing

Using pipx

pipx install --suffix=@10097 'poetry @ git+https://github.com/python-poetry/poetry.git@refs/pull/10097/head'
alias poetry=poetry@10097
poetry new foobar
cd foobar
poetry install
poetry env use 3.13

Using a container (podman | docker)

podman run --rm -i --entrypoint bash docker.io/python:latest <<EOF
set -xe 
python -m pip install --disable-pip-version-check -q git+https://github.com/python-poetry/poetry.git@refs/pull/10097/head
poetry new foobar
cd foobar
poetry env use 3.13
EOF

Summary by Sourcery

Tests:

  • Use mocked Python versions in tests.

Summary by Sourcery

Tests:

  • Use mocked Python versions in tests.

@abn abn requested a review from a team January 23, 2025 13:53
Copy link

sourcery-ai bot commented Jan 23, 2025

Reviewer's Guide by Sourcery

This pull request introduces the usage of the findpython library for Python version discovery, replacing Poetry's custom implementation. This change primarily affects the Python Manager, aiming to improve the accuracy and reliability of Python version detection.

Sequence diagram for Python version discovery process

sequenceDiagram
    participant Client
    participant Python
    participant FindPython
    participant ShutilWhichProvider

    Client->>Python: get_preferred_python(config, io)
    alt use-poetry-python is false
        Python->>Python: get_active_python()
        Python->>ShutilWhichProvider: find_pythons()
        ShutilWhichProvider-->>Python: python_list
        alt python found
            Python-->>Client: active_python
        else no python found
            Python->>FindPython: find('python')
            FindPython-->>Python: python_version
            Python-->>Client: python_instance
        end
    else use poetry python
        Python->>Python: get_system_python()
        Python-->>Client: system_python
    end
Loading

Class diagram for Python version management changes

classDiagram
    class Python {
        -_python: PythonVersion
        +executable: Path
        +version: Version
        +patch_version: Version
        +minor_version: Version
        +get_active_python()
        +from_executable(path)
        +get_system_python()
        +get_by_name(python_name)
        +get_preferred_python(config, io)
        +get_compatible_python(poetry, io)
    }

    class ShutilWhichPythonProvider {
        +create()
        +find_pythons()
    }

    class PythonVersionError {
    }

    class PythonVersionNotFoundError {
        +__init__(expected: str)
    }

    class NoCompatiblePythonVersionError {
        +__init__(expected: str, given: str)
    }

    class InvalidCurrentPythonVersionError {
        +__init__(expected: str, given: str)
    }

    PythonVersionNotFoundError --|> PythonVersionError
    NoCompatiblePythonVersionError --|> PythonVersionError
    InvalidCurrentPythonVersionError --|> PythonVersionError
    ShutilWhichPythonProvider --|> BaseProvider

    note for Python "Uses findpython library for version discovery"
Loading

File-Level Changes

Change Details Files
Replaced custom Python discovery logic with findpython library.
  • Removed custom Python version detection logic.
  • Added findpython as a dependency.
  • Implemented findpython to discover Python versions.
  • Updated Python manager to use findpython.
  • Added mocks for findpython in tests.
  • Adjusted tests to use mocked Python versions.
  • Removed usage of shutil.which for python discovery.
  • Removed usage of subprocess.check_output for python discovery.
src/poetry/utils/env/python/manager.py
src/poetry/utils/env/python/exceptions.py
src/poetry/utils/env/python/providers.py
src/poetry/utils/env/python/__init__.py
src/poetry/utils/env/env_manager.py
tests/utils/test_python_manager.py
tests/utils/env/test_env_manager.py
tests/conftest.py
tests/console/commands/test_new.py
tests/console/commands/test_init.py
tests/console/commands/env/test_use.py
tests/types.py
Added new fixtures for mocking findpython.
  • Added mocked_pythons fixture to provide a list of mocked Python versions.
  • Added mocked_pythons_version_map fixture to provide a map of mocked Python versions.
  • Added mock_findpython_find fixture to mock the findpython.find function.
  • Added mock_findpython_find_all fixture to mock the findpython.find_all function.
  • Added mocked_python_register fixture to register mocked Python versions.
  • Added with_mocked_findpython fixture to enable mocking of findpython.
  • Added without_mocked_findpython fixture to disable mocking of findpython.
tests/conftest.py
Refactored tests to use mocked Python versions.
  • Updated tests to use mocked_pythons fixture.
  • Updated tests to use mock_findpython_find fixture.
  • Updated tests to use mock_findpython_find_all fixture.
  • Updated tests to use mocked_python_register fixture.
  • Removed direct calls to subprocess.check_output.
  • Removed direct calls to shutil.which.
tests/utils/test_python_manager.py
tests/utils/env/test_env_manager.py
tests/console/commands/test_new.py
tests/console/commands/test_init.py
tests/console/commands/env/test_use.py
Introduced Python class to encapsulate Python version information.
  • Created Python class to represent a Python executable and its version.
  • Added methods to get active, system, and compatible Python versions.
  • Moved Python related exceptions to src/poetry/utils/env/python/exceptions.py.
  • Created ShutilWhichPythonProvider to find python executables in the path.
src/poetry/utils/env/python/manager.py
src/poetry/utils/env/python/exceptions.py
src/poetry/utils/env/python/providers.py
src/poetry/utils/env/python/__init__.py
Updated EnvManager to use the new Python class.
  • Modified EnvManager to use the Python class for Python version detection.
  • Updated EnvManager to use the Python class for creating virtual environments.
  • Removed direct calls to findpython from EnvManager.
src/poetry/utils/env/env_manager.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@abn abn added area/project/deps Related to Poetry's own dependencies area/python Related to detection of Python installations labels Jan 23, 2025
@abn abn force-pushed the feature/findpython branch from 1a92276 to c5081fc Compare January 23, 2025 15:48
@abn
Copy link
Member Author

abn commented Jan 23, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
@abn abn force-pushed the feature/findpython branch 2 times, most recently from e4fe342 to 28828ae Compare January 23, 2025 17:35
@abn abn force-pushed the feature/findpython branch from 28828ae to bb2665b Compare January 23, 2025 17:51
@abn abn force-pushed the feature/findpython branch 3 times, most recently from 651c337 to 5e2e702 Compare January 27, 2025 03:11
@abn
Copy link
Member Author

abn commented Jan 27, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
src/poetry/utils/env/python_manager.py Outdated Show resolved Hide resolved
@abn abn force-pushed the feature/findpython branch from 5e2e702 to 44fd55f Compare January 27, 2025 22:12
@abn abn requested a review from finswimmer January 27, 2025 22:14
@abn abn force-pushed the feature/findpython branch 3 times, most recently from c9d82b1 to efb8bbf Compare January 30, 2025 16:27
@abn abn enabled auto-merge (rebase) January 30, 2025 16:29
@abn abn force-pushed the feature/findpython branch 2 times, most recently from deb59d9 to 7799411 Compare February 1, 2025 17:25
@abn
Copy link
Member Author

abn commented Feb 1, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@abn abn requested a review from a team February 1, 2025 22:06
@abn abn force-pushed the feature/findpython branch from 7799411 to 2bd160c Compare February 2, 2025 12:45
@abn abn force-pushed the feature/findpython branch from 2bd160c to 54a0a13 Compare February 2, 2025 12:47
@abn abn requested a review from finswimmer February 2, 2025 12:48
@abn abn merged commit 73f4195 into python-poetry:main Feb 2, 2025
53 checks passed
@abn abn deleted the feature/findpython branch February 2, 2025 13:08
@abn abn mentioned this pull request Feb 2, 2025
@finswimmer finswimmer mentioned this pull request Feb 6, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/project/deps Related to Poetry's own dependencies area/python Related to detection of Python installations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants