FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(bigquery): add description for routine entities by HemangChothani · Pull Request #9785 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

feat(bigquery): add description for routine entities - #9785

Merged
tswast merged 2 commits into
googleapis:masterfrom
MaxxleLLC:bigquery_add_description_for_routine_entities
Nov 19, 2019
ConversationCommits (2)ChecksFiles changed
Merged

feat(bigquery): add description for routine entities#9785
tswast merged 2 commits into
googleapis:masterfrom
MaxxleLLC:bigquery_add_description_for_routine_entities

Conversation

HemangChothani commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

Fixes #9782

HemangChothani requested review from a team, plamut and tswast November 13, 2019 14:07
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 13, 2019

tseaver commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

/cc @shollyman

plamut suggested changes Nov 14, 2019
View reviewed changes

plamut left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

UPDATE: Nah, sorry, it does seem to work. I had to delete docs/generated directory as well (and not just docs/_build)

It seems that the description property is present in the generated docs, but in not the attributes summary table (unlike other Routine attributes).

Edit: Hmm, it might be a local cache issue, let me re-check. (nope, it is indeed missing from the attributes summary)

Comment thread bigquery/google/cloud/bigquery/routine.py Outdated

@property
def description(self):
"""Union[str, None]: Description of the routine (defaults to

plamut Nov 14, 2019 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

(nit)
Mypy docs recommend using the Optional shorthand instead of a Union with None.

Suggested change
"""Union[str, None]: Description of the routine (defaults to
"""Optional[str]: Description of the routine (defaults to

HemangChothani Nov 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ohk, should i change same in the other files which has description property like in dataset.py and model.py .

plamut Nov 14, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Strictly speaking it is not in scope of this PR, but if it's not too much work, it would be a welcome cleanup IMO. 👍

plamut approved these changes Nov 14, 2019
View reviewed changes

plamut 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

There are still usages of Union[foo, None], but with other attributes, thus let's leave that out for another time.

tswast merged commit 0b69ee0 into googleapis:master Nov 19, 2019
HemangChothani deleted the bigquery_add_description_for_routine_entities branch December 17, 2019 12:16
This was referenced Jan 29, 2020
Merged
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tswast tswast approved these changes
+1 more reviewer
plamut plamut approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BigQuery: add Description for Routine entities

5 participants

Back | FazBrowse Home | New Git URL