Skip to content

Emergent ruff error with 71.0 release #4473

Description

@jaraco

Although tests passed last week, after merging #4457 and cutting a release, tests started failing with this lint error:

>           raise RuffError(stdout.decode())
E           pytest_ruff.RuffError: setuptools/command/editable_wheel.py:302:39: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
E               |
E           300 |         for name in build.get_sub_commands():
E           301 |             cmd = self.get_finalized_command(name)
E           302 |             if name == "build_py" and type(cmd) != build_py_cls:
E               |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^ E721
E           303 |                 self._safely_run(name)
E           304 |             else:
E               |

.tox/py/lib/python3.11/site-packages/pytest_ruff/__init__.py:101: RuffError

Activity

  1. jaraco commented on Jul 17, 2024

    @jaraco
    MemberAuthor

    It seems it was bugfix release of ruff that enabled the new error:

     setuptools main 🐚 ruff check setuptools/command/editable_wheel.py
    All checks passed!
     setuptools main 🐚 pipx upgrade ruff
    ⚠️ Found a space in the home path. We heavily discourage this, due to multiple incompatibilities. Please check our docs for more information on
        this, as well as some pointers on how to migrate to a different home path.
    upgraded package ruff from 0.5.1 to 0.5.2 (location: /Users/jaraco/Library/Application Support/pipx/venvs/ruff)
     setuptools main 🐚 ruff check setuptools/command/editable_wheel.py
    setuptools/command/editable_wheel.py:302:39: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
        |
    300 |         for name in build.get_sub_commands():
    301 |             cmd = self.get_finalized_command(name)
    302 |             if name == "build_py" and type(cmd) != build_py_cls:
        |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^ E721
    303 |                 self._safely_run(name)
    304 |             else:
        |
    
    Found 1 error.
    
  2. jaraco commented on Jul 17, 2024

    @jaraco
    MemberAuthor

    Looks like the change was due to an unintended false negative (astral-sh/ruff#12300).

  3. jaraco commented on Jul 17, 2024

    @jaraco
    MemberAuthor

    Oh, gosh. There's more than one.

    >           raise RuffError(stdout.decode())
    E           pytest_ruff.RuffError: pkg_resources/tests/test_pkg_resources.py:282:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
    E               |
    E           280 |     # Check that the message portion contains the path.
    E           281 |     assert metadata_path in msg, str((metadata_path, msg))
    E           282 |     assert type(dist) == expected_dist_type
    E               |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
    E               |
    
    .tox/py/lib/python3.8/site-packages/pytest_ruff/__init__.py:101: RuffError
    
    
  4. reopened this on Jul 17, 2024
  5. jaraco commented on Jul 17, 2024

    @jaraco
    MemberAuthor

    I force pushed 6c6e2e1 over the previous fix.

  6. Avasam commented on Jul 17, 2024

    @Avasam
    Contributor

    @jaraco May I suggest locking Ruff versions just like we do mypy and I do for pyright in #4192 ? So that new Ruff violations don't "suddenly appear" ?

  7. added a commit that references this issue on Jul 17, 2024
    6c6e2e1
  8. jaraco commented on Jul 17, 2024

    @jaraco
    MemberAuthor

    @jaraco May I suggest locking Ruff versions just like we do mypy and I do for pyright in #4192 ? So that new Ruff violations don't "suddenly appear" ?

    I'd rather not. Pinning dependencies is a recipe for endless toil. In my experience, 95-99% of updates don't require any action, so adding a required action and static config change for each of those updates is very costly. Multiply that cost by the 100s of projects I maintain or the tens of thousands we all maintain and it's unsustainable.

    See #4025 (comment) and #1566 (comment) for previous discussions on pinning.

    Also, any change to the strategy for managing dependencies that's not specific to this project should probably be dealt with at the infrastructure level. For example, jaraco/skeleton#109 touches on the topic of pinning. Curiously, no one has opened up an issue in the skeleton to propose pinning dependencies. I'd be open to having that discussion and maybe summarizing my position based on the aforementioned references, but I'd be unlikely to be convinced.

    I stand by what I said in 1566 - if someone is willing to take on the toil of managing and maintaining the pins (either per-project or across projects), I'd be open to the prospect. But I've already encountered toil from the presence of dependabot (#4469).

    Do we have an owner for the pinning of mypy? We should add their nickname to a comment on the pin so we know whom to contact/assign when issues arise.

  9. Avasam commented on Jul 17, 2024

    @Avasam
    Contributor

    Given how you're not super hard set on "CI must pass / be green" anyway, I understand your preference for random failure over dependency management. And "following the skeleton" does make dependency management more tiresome in your case.

    Ruff and pyright tend to be updated often, and by their nature of being checkers that introduce new ways for the CI to fail in each update, they tend to cause these random failures quite often if left unpinned (but updating pins constantly isn't much better I guess, personally I wait for a dependabot/renovate PR to fail, or for a new feature to be needed, but I don't have the same multi-projects-wide restrictions).
    mypy is in a similar group where any update is likely to cause new failures, it's just not updated often.

    I appreciate the lengthy response and links to previous conversations :) (I don't have much to add to those)

    Do we have an owner for the pinning of mypy?

    No, but I guess this would either default to me (having added it), or to be unpinned. Given your stated preference, you can unpin it. If I have a PR that suddenly fails due to a new release, it might be confusing to a new contributor, but I personally would know how to handle it and create a separate PR if no one beats me to the punch.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions