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

[query] fix & improve pprint for hl.Struct #12901

Merged
merged 6 commits into from Apr 28, 2023
Merged

Conversation

danking
Copy link
Contributor

@danking danking commented Apr 18, 2023

CHANGELOG: hl.Struct now has a correct and useful implementation of pprint.

For structs with keys that were not identifiers, we produced incorrect repr output. For pprint, we just pprint'ed a dictionary (so you cannot even tell that the object was an hl.Struct). This PR:

  1. Fixes hl.Struct.__str__ to use the kwargs or dictionary representation based on whether the keys are Python identifiers.
  2. Teaches StructPrettyPrinter to first try to repr the struct (this is what the default pretty printer does)
  3. Teaches StructPrettyPrinter to properly pretty print a struct as an hl.Struct preferring the kwarg representation when appropriate.
  4. Teaches _same to use pretty printing when showing differing records.

CHANGELOG: `hl.Struct` now has a correct and useful implementation of pprint.

For structs with keys that were not identifiers, we produced incorrect `repr` output. For `pprint`,
we just `pprint`'ed a dictionary (so you cannot even tell that the object was an `hl.Struct`). This
PR:

1. Fixes `hl.Struct.__str__` to use the kwargs or dictionary representation based on whether the
   keys are Python identifiers.
2. Teaches `StructPrettyPrinter` to first try to `repr` the struct (this is what the default pretty
   printer does)
3. Teaches `StructPrettyPrinter` to properly pretty print a struct as an `hl.Struct` preferring the
   kwarg representation when appropriate.
4. Teaches `_same` to use pretty printing when showing differing records.
@danking danking assigned chrisvittal and unassigned tpoterba Apr 24, 2023
@danking
Copy link
Contributor Author

danking commented Apr 24, 2023

Lightening Tim's review load a bit.

@danking danking merged commit 948b1d9 into hail-is:main Apr 28, 2023
8 checks passed
danking added a commit to danking/hail that referenced this pull request May 8, 2023
I forgot to check in the tests for hail-is#12901. I found them on my filesystem today while cleaning up
untracked files.
danking added a commit that referenced this pull request May 11, 2023
I forgot to check in the tests for #12901. I found them on my filesystem
today while cleaning up untracked files.
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.

None yet

3 participants