Apply use-pathlib rules #10060

Secrus · 2025-01-16T13:31:54Z

Since we use os.path.join as a string glue method, I silenced those cases.

Summary by Sourcery

Refactor file operations to use pathlib.

Enhancements:

Replace open() with Path.read_text() and Path.write_text() for file I/O.
Use Path.mkdir() instead of os.mkdir.
Use Path.exists() instead of os.path.exists().
Use Path.is_symlink() instead of os.path.islink().
Use Path.unlink() instead of os.unlink().
Use Path.name instead of os.path.basename().

sourcery-ai · 2025-01-16T13:31:59Z

Reviewer's Guide by Sourcery

This pull request refactors file operations in the tests/packages/test_locker.py, tests/utils/env/test_env_manager.py, tests/console/commands/test_config.py, src/poetry/utils/helpers.py, src/poetry/utils/env/env_manager.py, tests/installation/test_chef.py, tests/utils/test_cache.py, src/poetry/inspection/info.py, src/poetry/packages/locker.py, tests/console/commands/env/helpers.py, tests/console/commands/self/test_add_plugins.py, tests/console/commands/self/test_self_command.py, tests/console/commands/test_init.py, tests/installation/test_wheel_installer.py, pyproject.toml, src/poetry/config/config.py, src/poetry/masonry/builders/editable.py, src/poetry/utils/env/virtual_env.py, tests/console/commands/self/test_remove_plugins.py, tests/installation/test_executor.py, tests/masonry/builders/test_editable_builder.py, and tests/utils/env/test_env.py files to use the pathlib module. It also updates the pyproject.toml to include the flake8-use-pathlib plugin.

Class diagram showing pathlib migration changes

classDiagram
    class Path {
      +exists()
      +chmod()
      +unlink()
      +is_symlink()
      +parent
      +readlink()
      +joinpath()
      +open()
    }

    class OldImplementation {
      +os.path.exists()
      +os.chmod()
      +os.unlink()
      +os.path.islink()
      +os.path.dirname()
      +os.readlink()
      +os.path.join()
      +open()
    }

    note for Path "New pathlib.Path implementation"
    note for OldImplementation "Previous os.path implementation"

    OldImplementation ..> Path : Migrated to

File-Level Changes

Change	Details	Files
Replaced file operations with pathlib equivalents.	Used `Path.read_text()` and `Path.write_text()` for file reading and writing. Used `Path.mkdir()` for directory creation. Used `Path.unlink()` for file deletion. Used pathlib methods for path manipulation. Used `Path.exists()` for file existence checks. Used `Path.is_symlink()` for symlink checks. Used `Path.open()` for opening files. Used `Path.joinpath()` for joining paths.	`tests/packages/test_locker.py` `tests/utils/env/test_env_manager.py` `src/poetry/utils/helpers.py` `src/poetry/utils/env/env_manager.py` `tests/installation/test_chef.py` `tests/utils/test_cache.py` `src/poetry/inspection/info.py` `src/poetry/packages/locker.py` `tests/console/commands/env/helpers.py` `tests/console/commands/self/test_add_plugins.py` `tests/console/commands/self/test_self_command.py` `tests/console/commands/test_init.py` `tests/installation/test_wheel_installer.py` `src/poetry/masonry/builders/editable.py` `src/poetry/utils/env/virtual_env.py` `tests/console/commands/self/test_remove_plugins.py` `tests/installation/test_executor.py` `tests/masonry/builders/test_editable_builder.py` `tests/utils/env/test_env.py`
Added `flake8-use-pathlib` plugin to `pyproject.toml`.	Included `flake8-use-pathlib` in the `extend-select` section.	`pyproject.toml`
Updated `os.path.join` usage in config command tests.	Added noqa comment to silence flake8-use-pathlib warning.	`tests/console/commands/test_config.py` `src/poetry/config/config.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.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a 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. You can also use
this command to specify where the summary should be inserted.

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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @Secrus - 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/env_manager.py

Secrus · 2025-01-16T14:33:30Z

@sourcery-ai review

sourcery-ai

Hey @Secrus - 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.}

sourcery-ai

We have skipped reviewing this pull request. It looks like we've already reviewed the commit fe0df70 in this pull request.

abn

Some initial comments.

abn · 2025-01-18T23:08:55Z

src/poetry/config/config.py

@@ -110,7 +110,7 @@ class Config:
        "virtualenvs": {
            "create": True,
            "in-project": None,
-            "path": os.path.join("{cache-dir}", "virtualenvs"),
+            "path": os.path.join("{cache-dir}", "virtualenvs"),  # noqa: PTH118


Any reason why we can't replace this?

from what I understand it would be str(Path(...)) which doesn't look like a good use of resources, while os.path operates on strings all the way

Add a comment maybe to avoid someone swapping it later.

But I reckon since it's not always a valid path (eg: template) it's okay.

abn · 2025-01-18T23:13:22Z

src/poetry/utils/env/env_manager.py

@@ -493,14 +493,14 @@ def create_venv(
        # but others can symlink *to* the venv Python,
        # so we can't just use sys.executable.
        # So we just check every item in the symlink tree (generally <= 3)
-        p = os.path.normcase(sys.executable)
+        p = Path(os.path.normcase(sys.executable))


Is normcase required with Path?

abn · 2025-01-18T23:15:16Z

src/poetry/utils/env/env_manager.py

            paths.append(p)

        p_venv = os.path.normcase(str(venv))
-        if any(p.startswith(p_venv) for p in paths):
+        if any(str(p).startswith(p_venv) for p in paths):


Shouldn't this use relative?

abn · 2025-01-18T23:16:43Z

tests/console/commands/self/test_add_plugins.py

@@ -194,7 +194,7 @@ def test_add_existing_plugin_warns_about_no_operation(
    installed: TestRepository,
 ) -> None:
    pyproject = SelfCommand.get_default_system_pyproject_file()
-    with open(pyproject, "w", encoding="utf-8", newline="") as f:
+    with pyproject.open("w", encoding="utf-8", newline="") as f:


Can we use write_text? Similar in a few other places.

Only after dropping support for Python 3.9, I assume:

Changed in version 3.10: The newline parameter was added.

https://docs.python.org/3/library/pathlib.html#pathlib.Path.write_text

Exactly this. I can add a TODO for when we drop 3.9 to change that

sourcery-ai bot reviewed Jan 16, 2025

View reviewed changes

src/poetry/utils/env/env_manager.py Outdated Show resolved Hide resolved

Secrus marked this pull request as draft January 16, 2025 14:00

Secrus force-pushed the use-pathlib branch from c3d4fdc to 69a44f5 Compare January 16, 2025 14:14

Apply use-pathlib rules

fe0df70

Secrus force-pushed the use-pathlib branch from 69a44f5 to fe0df70 Compare January 16, 2025 14:22

sourcery-ai bot reviewed Jan 16, 2025

View reviewed changes

Secrus requested a review from a team January 18, 2025 16:44

Secrus marked this pull request as ready for review January 18, 2025 16:45

sourcery-ai bot reviewed Jan 18, 2025

View reviewed changes

abn reviewed Jan 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply use-pathlib rules #10060

Apply use-pathlib rules #10060

Secrus commented Jan 16, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jan 16, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Secrus commented Jan 16, 2025

sourcery-ai bot left a comment

sourcery-ai bot left a comment

abn left a comment

abn Jan 18, 2025

Secrus Jan 19, 2025

abn Jan 20, 2025

abn Jan 18, 2025

abn Jan 18, 2025

abn Jan 18, 2025

radoering Jan 19, 2025

Secrus Jan 19, 2025

Apply use-pathlib rules #10060

Are you sure you want to change the base?

Apply use-pathlib rules #10060

Conversation

Secrus commented Jan 16, 2025 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Jan 16, 2025 • edited Loading

Reviewer's Guide by Sourcery

Class diagram showing pathlib migration changes

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Secrus commented Jan 16, 2025

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot left a comment

Choose a reason for hiding this comment

abn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Secrus commented Jan 16, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jan 16, 2025 •

edited

Loading