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

[Feature Request] Sort dict keys #10085

Open
Tatsh opened this issue Feb 22, 2024 · 4 comments
Open

[Feature Request] Sort dict keys #10085

Tatsh opened this issue Feb 22, 2024 · 4 comments
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule

Comments

@Tatsh
Copy link

Tatsh commented Feb 22, 2024

Would be nice and I did not find this feature already being requested.

ESLint has this feature: sort-keys. Should have options like case-sensitivity, minimum keys, natural, asc/desc. # noqa: ... should disable the requirement and stop the auto-fixer.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Feb 23, 2024
@AlexWaygood
Copy link
Member

Thanks for the feature request! For reference, this is similar to a few issues that have been opened before:

However, I think we should keep this open. The first two of those issues were closed in favour of #1198, which was closed out without implementing this specific feature. The third is different enough to this that it could be considered separate.

I'm not sure I'd want to turn on this rule in my Python projects, since Python dictionaries maintain insertion order when you iterate through them (they have been guaranteed to do so by the language spec since Python 3.7), and I've written code in the past that's relied on that useful property. That means that the autofix proposed here has the potential to break working code. The only Python builtin collections where it would be safe to autosort the collection are sets and frozensets, since those are the only two that do not maintain insertion order.

Nonetheless, it does seem like there's a fair amount of user demand for this feature, and I gather the ESLint rule also has the potential to change the iteration order of a JavaScript object. So I think we should consider having a rule like this. If we're sorting dictionary keys, though, we may as well also sort list, tuples and sets, since we'd already be doing something pretty unsafe by sorting dictionary keys.

One open question is whether enabling this rule should sort all collections? isort has a feature where collections that are marked with an # isort: unique-list comment are deduplicated and sorted -- we could possibly consider doing something similar.

  • The advantage is that it neatly works around the safety issues (we'd only be sorting collections where the user has explicitly asked us to do so).
  • The disadvantages are:
    • It's another pragma comment for users to have to remember/more configuration is really confusing
    • It's more verbose to have to add a comment next to every collection you want to be sorted like this.

@AlexWaygood AlexWaygood added the needs-decision Awaiting a decision from a maintainer label Feb 26, 2024
@MichaReiser
Copy link
Member

we may as well also sort list, tuples and sets, since we'd already be doing something pretty unsafe by sorting dictionary keys.

It would be worth having different rules, at least for lists. I've used object sorting in very large code bases in JS with very little need to suppress the rules because it's uncommon that the ordering matters (at least in the projects I worked). However, that's different for lists where ordering is more likely to be semantically meaningful (or I would have used a set).

@JelleZijlstra
Copy link
Contributor

I would like this specifically for set (I realize that's not what the issue is about). Sets don't have insertion ordering, so unlike with dicts or lists, there isn't as much of a concern about changing behavior. For example, it would be nice if Ruff sorted this set so it's easier to keep track of which letters are in it.

@AlexWaygood
Copy link
Member

I would like this specifically for set (I realize that's not what the issue is about). Sets don't have insertion ordering, so unlike with dicts or lists, there isn't as much of a concern about changing behavior. For example, it would be nice if Ruff sorted this set so it's easier to keep track of which letters are in it.

Yeah, I agree that it makes much more sense for sets than any other data structure. I wonder if this should be multiple rules — or one, configurable rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants