Skip to content

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Jul 9, 2025

Following (#2803), two (PD002) ignores are no longer needed

Previous

result.reset_index(inplace=True) # noqa: PD002

result_complex.reset_index(inplace=True) # noqa: PD002

main

result.reset_index(inplace=True)

result_complex.reset_index(inplace=True)

Notes

pre-commit autoupdate enforces the latest version of ruff in ci, but we don't have anything syncing this with pyproject.toml.
A solution for now is just to leave it unpinned, so at least it'll install the latest when updating a local venv

Those lines have moved in (#2680), and still require the (PD002) ignore - so I think the removal is related to ruff's type inference changing under-the-hood

df.reset_index(inplace=True) # noqa: PD002

Following (#2803), two (`PD002`) ignores are no longer needed
- https://github.com/narwhals-dev/narwhals/blob/54bf3d3ec77092d4babc970f245efc2185274e96/narwhals/_pandas_like/group_by.py#L217
- https://github.com/narwhals-dev/narwhals/blob/54bf3d3ec77092d4babc970f245efc2185274e96/narwhals/_pandas_like/group_by.py#L264

`pre-commit` autoupdate enforces the latest version of `ruff` in ci, but we don't have anything syncing this with `pyproject.toml`.
A solution for now is just to leave it unpinned, so at least it'll install the latest when updating a local `venv`
@dangotbanned dangotbanned added ci internal fix dependencies Pull requests that update a dependency file labels Jul 9, 2025
@dangotbanned dangotbanned marked this pull request as ready for review July 9, 2025 14:07
@MarcoGorelli
Copy link
Member

thanks - not sure tbh, i think we should rather just have it in pre-commit, and if anyone doesn't want to use pre-commit then it's on them to self-manage

@dangotbanned
Copy link
Member Author

thanks - not sure tbh, i think we should rather just have it in pre-commit, and if anyone doesn't want to use pre-commit then it's on them to self-manage

I'm using pre-commit and by not having ruff declared in our dependencies - VSCode will default to what is used in https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff

The extension ships with ruff==0.12.0.

So on unrelated PRs, as soon as I merge (#2803) I now get warnings from a problem introduced in 0.12.2.
If I decide to self-manage, I now have to keep everything in sync with pre-commit.
If I don't want to self-manage, I can't get feedback in VSCode and have to rely on pre-commit which is much slower.

Is there some cost to adding it to dev that I'm missing?
Just from sampling our upstream and downstream dependencies, this is very common:

I just want to be able to auto-fix stuff in my IDE, without needing to concern myself with manually syncing dependencies 😅

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

you'd still need to keep it in sync if there's a new ruff release before the pre-commit autoupdate job runs

but, sure, if it eases dev workflow, no objections

@dangotbanned
Copy link
Member Author

@dangotbanned dangotbanned merged commit cc22378 into main Jul 9, 2025
40 of 41 checks passed
@dangotbanned dangotbanned deleted the dev-add-ruff branch July 9, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci dependencies Pull requests that update a dependency file fix internal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants