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

Add callable sort key functionality to sort_keys #666

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sparr
Copy link

@sparr sparr commented Sep 20, 2022

Currently any truthy value of sort_keys will send the mapping pairs through sorted() before representing the mapping. This PR extends that functionality so that callable values of sort_keys will be passed as the key parameter of sorted(). A test is included for coverage and illustration purposes.

For my own purposes, I have a few use cases in mind, each to be applied in different scenarios:

  1. sort_keys = lambda k: k.upper() for a case insensitive sort that I expect to make more sense to a human reader.
  2. Similar to the test example, but with explicit ordering of the "sort first" keys, so I can get critical keys like "name" and "path" to the top where a human reader will see them first.
  3. Sorting based on the length/size of the value (not key) so longer dicts/arrays don't interrupt the visual flow of shorter values

This introduces a minor naming confusion issue, in that the sort applies not just to the keys but to (key,value) pairs produced by .items(). That was always the case, but wouldn't have been noticeable previously since dict keys are unique so values were never considered as part of the default sort. This requires sort_keys functions to access the key as x[0] instead of just x which may be counterintuitive. This could be avoided by passing only the keys to the key function instead of the pairs, but that would break my third use case above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant