-
-
Notifications
You must be signed in to change notification settings - Fork 574
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
Why does isort sort packages ending with a number oddly? #1732
Comments
i think that the problem lies here: Line 118 in 89e1e2c
bob2.apples2 and bob.apples would be ["bob", "2", ".apples", "2"] and ["bob.apples"] respectively, and the first list is smaller than the second so the bob2.apples2 is sorted before bob.apples
|
Thanks @anirudnits. I don't understand what that code is designed to handle? |
@bobwalker99, sorry this behavior didn't match your expectations, and I can see why you might expect the other! What isort is trying to handle by sorting numbers in this way, is natural number sorting:
which the naive Python sorting turns into
Like you alluded to, both cases are corner cases, however in practice these numbered imports have popped up more commonly in codebases that use isort, and to this date we haven't had any requests for the other kind of case. Does the difference matter to you, would an option here be useful? Thanks! ~Timothy |
Hi Timothy, many thanks for the quick response, I understand what’s going on now. A config option for this would be really good - I’m happy to submit a PR, but I might need a nudge in the right direction of how you think it needs to work. |
Hi @bobwalker99, Would happily accept such a PR! I think right now, all sorting gets routed to Line 99 in f074239
It might be worth introducing a new central sorting function, that is config aware, that then checks if natural sorting is set or not, and if not goes to the Python standard way of sorting, and rerouting all the existing calls through that new function. This would make it easier as well to provide a way for users to write fully custom sorters, which has been an ask before as well. Happy to help in any way I can, let me know what you think ~Timothy |
Hi @timothycrosley, many thanks for the guidance. I've just started thinking about this a bit more - I'm not sure that toggling this behaviour is actually what we'd want? I agree it's reasonable to want modules sorted this way:
But the addition of sub-packages and modules of the same name without numbers seems to actually break the output? e.g.:
Becomes this:
If I'd set a "natural language sorting" setting to True, I think I'd expect Nonetheless, your pluggable sorter suggestion is a good one, so I'll have a go at implementing that. If my use case has an audience of 1, I can then always write something that will do what I'm expecting ;-) ! Appreciate your input on this, all the best, Bob |
Hi @timothycrosley would you mind taking a look at #1752 to check I'm on the right track before I go too far with it please? It's very basic inasmuch as it just expects a string descriptor for a custom module/function and will just failover to I'm worried about the amount of testing that might be required if some custom function was employed in conjunction with other config settings, and how I would even test that? |
Closing as this has been fixed by @bobwalker99's pull request: #1756 which was just merged in develop 🎉. A new PyPI release should appear in the coming days that contains the new sort mode flag |
Given these, sorted alphanumerically:
isort reorders them like this:
Is there a config setting that controls this behaviour? I don't understand the behaviour or the intention here.
Python by default sorts them as I would expect:
So this must be intentional? I've experimented with
force_alphabetical_sort_within_sections
andlexicographical
andlength_sort
but I can only get accidental success which adversely affects other behaviour.(Appreciate this is a contrived example, but it mirrors an actual scenario we have in our production code).
The text was updated successfully, but these errors were encountered: