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

New rule category XP, implement not-array-agnostic-numpy (XP001) #8910

Closed
wants to merge 8 commits into from

Conversation

lucascolley
Copy link

@lucascolley lucascolley commented Nov 29, 2023

Summary

Closes gh-8615

  • New rule category XP
  • New rule NotArrayAgnosticNumPy

Aiming for violations for all non-standard NumPy functions and methods and uses of them. Some of these will have standard aliases or alternatives. We should try to catch as many of the 'compatible' and 'strictness' changes of https://numpy.org/doc/stable/reference/array_api.html as possible (the 'breaking' changes should be caught separately - by NP201 or perhaps NP202 is also needed).

Test Plan

WIP

@zanieb zanieb added the plugin Implementing a known but unsupported plugin label Nov 29, 2023
@lucascolley
Copy link
Author

lucascolley commented Dec 6, 2023

Hey @charliermarsh, here's where I'm at currently. I've included the raw naming differences as replacements so far.

To-do:

  • Include the standard extension modules to check against numpy.linalg and numpy.fft
  • Start work on tests

Are these things possible?

  • Detecting methods of numpy.ndarray e.g. we want to change a.astype(...) to np.astype(a, ...) when a is of type numpy.ndarray
  • Detecting whether certain arguments are passed e.g. if numpy.transpose(a) is used without the axes keyword then it should be replaced with numpy.permute_dims(a, axes=range(a.ndim)[::-1]). If it is provided, then a straight swap for permute_dims is needed
  • Detecting the dtypes used with operators e.g.

"Operators (like +) with Python scalars only accept matching scalar types - For example, <int32 array> + 1.0 is not allowed.

  • Changing positional arguments to keyword arguments and vice versa

@MichaReiser
Copy link
Member

Thank you @lucascolley for this rule proposal and implementation. I really like the idea of it. I think such a rule would be very helpful for our users. Unfortunately, Ruff doesn't have a good concept today for very opinionated rules or rules that intentionally restrict the allowed APIs. Ruff also lacks a good default rule set, and, unfortunately, many users assume that ALL is our recommended rule set. That means users using --select ALL would automatically opt into this new restricting rule, and they do not necessarily have the experience to judge whether the pattern flagged by this rule is indeed a "problem" for them or if they are better off by ignoring the rule. We want to hold off from merging restricting rules until #1774 is complete. We're actively working on it, and we hope to have a good place for rules like the one you proposed here.

I'm sorry that it took us so long to give that feedback to you, and I hope we can get this rule landed once #1774 is completed. Thanks again.

@lucascolley
Copy link
Author

lucascolley commented Mar 28, 2024

Thanks for the reply @MichaReiser , no worries! Happy to return to this once gh-1774 is resolved. I agree that this should be explicitly opt-in at all times.

That means users using --select ALL would automatically opt into this new restricting rule

Does that include preview rules? If so, should it not?

@MichaReiser
Copy link
Member

Does that include preview rules? If so, should it not?

It does include preview rules when running ruff with preview = true (you have to opt into preview). We considered accepting rules as preview only but we decided against it because accepting rules without a clear path to stabilization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Implementing a known but unsupported plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: new rule category for array-agnostic code (XP)
4 participants