ruff icon indicating copy to clipboard operation
ruff copied to clipboard

Ruff rule to remove six

Open inoa-jboliveira opened this issue 10 months ago • 6 comments

Proposal: creation of rules to remove usages of the six module, replacing with the Python 3 only equivalent. This would align with the UP rules, but I don't know if they would be allowed in there or simply into the RUF code. The idea is to have fixable rules for the most common features of six and help people migrate with more confidence. There should also exist a catch-all rule just checking if six was imported.

  • UP900 Deprecate six (not fixable) .
    • E.g. import six -> "Usage of six is discouraged on applications not supporting Python 2"
  • UP901 Replace types (e.g. six.string_types, six.text_type, six.binary_type, six.integer_types) with corresponding Python 3 types.
    • E.g.: isinstance(foo, six.string_types) -> isintance(foo, (str,))
    • Note: arguably this is the most used feature of six
  • UP902 Replace six.iter* with the corresponding method.
    • E.g.: for k, v in six.iteritems(foo): -> for k, v in foo.items():
  • UP903 Replace six.print_ with print.
    • E.g.: six.print_("hello") -> print("hello")
  • UP904 Replace six.PY2, PY3, ... with direct checks.
    • E.g.: if six.PY3: -> if sys.version_info[0] == 3:
    • Note: this is a quite important one because six.PY3 is a source of bugs -- see YTT202
    • Could also warn the user that the check is no longer be necessary since all code is running on Python 3

inoa-jboliveira avatar Apr 22 '24 14:04 inoa-jboliveira

There's some of this in the UP rules already (like UP029 removes six.callable and others that are now builtins), but I think I intentionally cut scope on other six rules because I wanted to avoid adding and maintaining complex rules that would only be relevant for one-time 2-to-3 transitions (as opposed to rules that are useful for ongoing Python 3 development).

charliermarsh avatar Apr 22 '24 23:04 charliermarsh

I'm open to adding some of these selectively under the UP category but it's a bit of a balance -- I don't want to add and maintain a bunch of complex six rules since the value is diminishing over time.

charliermarsh avatar Apr 22 '24 23:04 charliermarsh

I share the sentiment with @charliermarsh. If the primary target group is python 2 users, then we should not implement the rule according to our rule acceptance guidelines.

UP900 can be achieved by using banned-api

MichaReiser avatar Apr 23 '24 07:04 MichaReiser

Hi @charliermarsh and @MichaReiser, thank you for the replies.

The point here is not serve people upgrading from python 2, it is to drop the old compatibility layer many projects kept for a long time even now supporting most modern python versions only.

Same thing with UP025 where you replace "u" prefix of strings which is leftover from years of legacy support.

I have a repo with over 1M LOC of python and it took a while to remove everything, but then I noticed many dependencies still imported six even if they no longer supported python 2.

This means having these rules could speed up projects from dropping this layer (again same purpose as many of the UP rules).

If you feel this doesn't bring enough benefit, you may close the issue

inoa-jboliveira avatar Apr 23 '24 11:04 inoa-jboliveira

Related:

https://github.com/astral-sh/ruff/pull/2049#discussion_r1090077646 https://github.com/astral-sh/ruff/pull/2332

eli-schwartz avatar May 07 '24 20:05 eli-schwartz

If it's possible to reimplement via more generic rewriting methods that didn't exist at the time, I think that would be quite convenient as IMO the UP rules are mostly a matter for oneshot actions, and six isn't particularly special. I think the only real reason in the past for not covering these rules effectively boiled down to implementation complexity and no one available to do the work?

eli-schwartz avatar May 07 '24 20:05 eli-schwartz