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: stable as_json #310

Closed
patrick91 opened this issue Aug 1, 2024 · 6 comments
Closed

feature: stable as_json #310

patrick91 opened this issue Aug 1, 2024 · 6 comments
Assignees
Labels
feature New feature or request

Comments

@patrick91
Copy link

Hi there! I have not investigated this much, but looks like using as_json doesn't always return the same data (or at least not in the same order) see this diff: strawberry-graphql/strawberry.rocks@fc70f2a

For my use case I'll do some sorting, but I was wondering if this could be a feature of Griffe 😊

@patrick91 patrick91 added the feature New feature or request label Aug 1, 2024
@patrick91 patrick91 changed the title feature: stable as json feature: stable as_json Aug 1, 2024
@pawamoy
Copy link
Member

pawamoy commented Aug 2, 2024

113,475 additions, 113,475 deletions not shown because the diff is too large. Please use a local Git client to view these changes.

Ouch 😅

Yes that's a good idea, we can probably use json.dumps(..., sort_keys=True) 🙂

@patrick91
Copy link
Author

@pawamoy I was using sort_keys, but then the order of the members would change too, not sure why :D

@pawamoy
Copy link
Member

pawamoy commented Aug 2, 2024

Ah 🤔 I'll investigate. Members order should definitely stay the same. Unless you're using dynamic analysis and inspect.getmembers doesn't return members in the same order every time 🤔

@pawamoy
Copy link
Member

pawamoy commented Aug 7, 2024

Can you tell me (again? sorry if I forgot) how you dump and reload the data? Or just point me at the code you're using to dump/load?

@pawamoy
Copy link
Member

pawamoy commented Aug 7, 2024

I can't replicate the members dis-ordering issue across multiple invocations of Griffe 🤔

@pawamoy
Copy link
Member

pawamoy commented Aug 7, 2024

Note that members are dumped in the order they are found in the sources. If you move objects around, the order in the JSON dump will also change 🙂 Maybe we shouldn't do that and sort alphabetically instead, since we either:

  • have line numbers information and therefore can sort again as found in sources
  • don't have line numbers information and that means sources could not be found, so source order doesn't make sense

By the way this source ordering is completely implicit and relies on default ordering of Python dicts since Python 3.7 (I think it's 3.7).

And if we don't need the initial, implicit order of members, maybe we can even stop using a list and use a dict when dumping them instead 🤷 That would by the way make the JSON more readable (for example, no need to expand an item in Firefox to see the member's name, all names would be displayed right there in the tree without having to expand anything).

pawamoy added a commit that referenced this issue Aug 14, 2024
@pawamoy pawamoy closed this as completed Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants