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

Output could use ordered collections to keep it consistently ordered with input #25

Closed
esuomi opened this issue Jun 17, 2022 · 4 comments
Labels
closed: pr welcome Closed because we have decided not to prioritize it but PRs are still welcome enhancement New feature or request help wanted Extra attention is needed

Comments

@esuomi
Copy link

esuomi commented Jun 17, 2022

Eg. https://github.com/clj-commons/ordered provides ordered collections, and the idea really is that there's no reshuffling of especially KV pairs in maps to make it even easier to spot differences between what was provided and what the diff was.

@alysbrooks
Copy link
Member

This is an interesting idea, and I agree that it would make the output easier to interpret. I haven't noticed deep-diff output being out of order, possibly because I mostly use deep-diff2 through Kaocha, so I don't see the original order. Is there a particular situation where this came up in?

I don't how optimized ordered collection libraries are or how big of dependencies they are, so I wonder if the best way is to make it configurable, require the user to provide the dependency, or both.

@plexus
Copy link
Member

plexus commented Jun 17, 2022

The thing is that this will only help if people are themselves using ordered collections in their code, since otherwise the order we read them in is not deterministic. For small maps clojure maintains source order (PersistentArrayMap). Once you get to bigger sizes it automatically switches to PersistentHashMap, and you can no longer depend on any ordering.

@esuomi
Copy link
Author

esuomi commented Jun 18, 2022

@alysbrooks No particular situation, this was literally just a Friday thought :)

I wonder if the sorting could be applied after diffing? After all it's more of a view thing than anything.

@alysbrooks alysbrooks added enhancement New feature or request help wanted Extra attention is needed closed: pr welcome Closed because we have decided not to prioritize it but PRs are still welcome labels Jul 5, 2023
@alysbrooks
Copy link
Member

alysbrooks commented Jul 5, 2023

I think there are two ideas here:

  • Add support for flatland.ordered collections. This should be done in a bring-your-own-dependency fashion—we don't want to pull in this library if people aren't using it.
  • Sort. This can be probably be done by the library user but isn't straightforward since the Deletion and Insertion objects don't implement Comparable.

I think we'd welcome a PR for either.

The main reason to not accept support for flatland.ordered is that it could be a maintenance burden for a feature very few people use. On the other hand, that library appears to be very stable. We'd definitely want tests for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: pr welcome Closed because we have decided not to prioritize it but PRs are still welcome enhancement New feature or request help wanted Extra attention is needed
Projects
Status: 🙈Out of Scope
Development

No branches or pull requests

3 participants