Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: error if eval()-ing repr(SchemaField) - #1046

Merged
tswast merged 5 commits into
googleapis:mainfrom
plamut:iss-1041
Nov 4, 2021
Merged

fix: error if eval()-ing repr(SchemaField)#1046
tswast merged 5 commits into
googleapis:mainfrom
plamut:iss-1041

Conversation

@plamut

Copy link
Copy Markdown
Contributor

Fixes#1041.

If policy tags are not set, the corresponding key should be None to denote a missing value. An empty tuple should only be used when the schema has explicitly empty PolicyTagList assigned.

PR checklist:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@plamut
plamut requested review from a team and tswastNovember 4, 2021 10:15
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Nov 4, 2021
@product-auto-labelproduct-auto-labelBot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Nov 4, 2021
evaled_field = eval(field_repr)

assert field == evaled_field

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, do we need a round-trip test for the case where there is a policy tuple?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not hurt, I'll add it.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess what, that extra test actually revealed deeper issues in the repr() of SchemaField and PolicyTagList.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W00t!

@plamutplamut added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 4, 2021
@plamut
plamut requested a review from tseaverNovember 4, 2021 15:14
@plamutplamut removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 4, 2021

@tswasttswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@tswast
tswast merged commit 13ac860 into googleapis:mainNov 4, 2021
@plamut
plamut deleted the iss-1041 branch November 4, 2021 21:10
abdelmegahedgoogle pushed a commit to abdelmegahedgoogle/python-bigquery that referenced this pull request Apr 17, 2023
* fix: error if eval()-ing repr(SchemaField)
* Make repr(PolicyTagList) evaluable
* Fix SchemaField repr with policy tags
The repr() should be evaluable.
Co-authored-by: Tres Seaver <tseaver@palladion.com>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SchemaField cannot initialize from __repr__

3 participants

@plamut@tseaver@tswast