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

Emergent ruff error with 71.0 release #4473

Closed
jaraco opened this issue Jul 17, 2024 · 7 comments
Closed

Emergent ruff error with 71.0 release #4473

jaraco opened this issue Jul 17, 2024 · 7 comments

Comments

@jaraco
Copy link
Member

jaraco commented Jul 17, 2024

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
@jaraco
Copy link
Member Author

jaraco commented Jul 17, 2024

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.

@jaraco
Copy link
Member Author

jaraco commented Jul 17, 2024

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

@jaraco jaraco closed this as completed in f2cd90f Jul 17, 2024
@jaraco
Copy link
Member Author

jaraco commented Jul 17, 2024

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

@jaraco jaraco reopened this Jul 17, 2024
@jaraco jaraco closed this as completed in 6c6e2e1 Jul 17, 2024
@jaraco
Copy link
Member Author

jaraco commented Jul 17, 2024

I force pushed 6c6e2e1 over the previous fix.

@Avasam
Copy link
Contributor

Avasam commented Jul 17, 2024

@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" ?

@jaraco
Copy link
Member Author

jaraco commented Jul 17, 2024

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

@Avasam
Copy link
Contributor

Avasam commented Jul 17, 2024

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.

@Avasam Avasam mentioned this issue Jul 20, 2024
2 tasks
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

No branches or pull requests

2 participants