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

automl: add base model samples for automl ga #2609

Merged
merged 8 commits into from
Dec 20, 2019

Conversation

nnegrey
Copy link
Contributor

@nnegrey nnegrey commented Dec 12, 2019

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 12, 2019
@nnegrey nnegrey marked this pull request as ready for review December 18, 2019 23:26
@nnegrey nnegrey requested a review from a team as a code owner December 18, 2019 23:26
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

Just nits, LGTM. It looks like you ran black over your code, the following styles I strongly disagree with:

  • Double quotes
  • Additional newlines wrapping parenthesis

The lint check should no longer enforce these styles but you're welcome to adopt them too, just do it consistently.

Also note that by default black will wrap lines at 88 columns, I prefer 80 or 79 as per the authoring guide for python.

model_full_id = client.model_path(project_id, "us-central1", model_id)
response = client.delete_model(model_full_id)

print("Model deleted. {}".format(response.result()))
Copy link
Contributor

Choose a reason for hiding this comment

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

(Optional) As per the Authoring guide, I prefer single quotes over double.


client = automl.AutoMlClient()
# Get the full path of the model.
model_full_id = client.model_path(project_id, "us-central1", model_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

The double quotes look a bit odd.

automl/cloud-client/deploy_model_test.py Outdated Show resolved Hide resolved
automl/cloud-client/get_model_evaluation.py Outdated Show resolved Hide resolved
# Get complete detail of the model evaluation.
response = client.get_model_evaluation(model_evaluation_full_id)

print("Model evaluation name: {}".format(response.name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Something about the quotes looks wrong.

print("List of model evaluations:")
for evaluation in client.list_model_evaluations(model_full_id, ""):
print("Model evaluation name: {}".format(evaluation.name))
print(
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird print style blocks

print("\tseconds: {}".format(evaluation.create_time.seconds))
print("\tnanos: {}".format(evaluation.create_time.nanos / 1e9))
print(
"Evaluation example count: {}".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird double quotes

automl/cloud-client/list_model_evaluations_test.py Outdated Show resolved Hide resolved
from google.cloud import automl

# TODO(developer): Uncomment and set the following variables
# project_id = 'YOUR_PROJECT_ID'
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're adopting double-quotes, use them consistently.

automl/cloud-client/undeploy_model_test.py Outdated Show resolved Hide resolved
@nnegrey
Copy link
Contributor Author

nnegrey commented Dec 19, 2019

Yea, I usually run black *.py -l 79
Yea, I'm not a fan of the two nits you pointed out.

Mostly I just started using black after I noticed the generated libraries use it, so I figured I'd conform.

@nnegrey
Copy link
Contributor Author

nnegrey commented Dec 19, 2019

Happy to fix the black formatting if we aren't adopting it in our style.

@gguuss
Copy link
Contributor

gguuss commented Dec 19, 2019

Mostly I just started using black after I noticed the generated libraries use it, so I figured I'd conform.

Meh, there are a lot of people on the team who are in favor of using Black despite all of it's stylistic shortcomings, just because it is easy to run. I'm ok with this style until we have an alternative for auto-formatting. I may look into customizing out the Black features that are triggering my style preferences in particular:

  • Double-quotes
  • The extra newlines at the start / end of ( ... ) blocks
  • Column wrapping at 88
  • The extra ',' characters added to the last members of function definitions on continuations
  • Newlines added to closing )'s

And any others that I find. Ideally, the auto-formatter we adopt should be configurable to our styles.

@nnegrey nnegrey merged commit f0a4963 into master Dec 20, 2019
@nnegrey nnegrey deleted the automl-ga-base-model-samples branch December 20, 2019 15:53
busunkim96 pushed a commit to busunkim96/python-automl that referenced this pull request Aug 7, 2020
…tform/python-docs-samples#2609)

* automl: add base model samples for automl ga

* Add tests for each file, provide a unique model for each python version for kokoro

* Fix version check

* Update tests and format

* Update deploy_model_test.py

* Update license headers, ensure double quotes is used everywhere, leave black formatting
busunkim96 pushed a commit to googleapis/python-automl that referenced this pull request Aug 11, 2020
…tform/python-docs-samples#2609)

* automl: add base model samples for automl ga

* Add tests for each file, provide a unique model for each python version for kokoro

* Fix version check

* Update tests and format

* Update deploy_model_test.py

* Update license headers, ensure double quotes is used everywhere, leave black formatting
arbrown pushed a commit that referenced this pull request Nov 17, 2022
* automl: add base model samples for automl ga

* Add tests for each file, provide a unique model for each python version for kokoro

* Fix version check

* Update tests and format

* Update deploy_model_test.py

* Update license headers, ensure double quotes is used everywhere, leave black formatting
dandhlee pushed a commit that referenced this pull request Nov 17, 2022
* automl: add base model samples for automl ga

* Add tests for each file, provide a unique model for each python version for kokoro

* Fix version check

* Update tests and format

* Update deploy_model_test.py

* Update license headers, ensure double quotes is used everywhere, leave black formatting
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Oct 21, 2023
…tform/python-docs-samples#2609)

* automl: add base model samples for automl ga

* Add tests for each file, provide a unique model for each python version for kokoro

* Fix version check

* Update tests and format

* Update deploy_model_test.py

* Update license headers, ensure double quotes is used everywhere, leave black formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants