Skip to content
This repository has been archived by the owner on Sep 9, 2023. It is now read-only.

feat: adds ValueConverter utility and demo samples #108

Merged
merged 16 commits into from Dec 18, 2020
Merged

Conversation

telpirion
Copy link
Contributor

This PR includes the following:

  • a ValueConverter class, that provides methods for converting Message types to/from protobuf.Value objects
  • three samples that demonstrate the usage of the enhanced types and ValueConverter class

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 4, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 4, 2020
@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #108 (980e089) into master (f0145cd) will decrease coverage by 11.99%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #108       +/-   ##
=============================================
- Coverage     76.57%   64.57%   -12.00%     
+ Complexity      606      530       -76     
=============================================
  Files            48       49        +1     
  Lines          5617     3263     -2354     
  Branches         69       14       -55     
=============================================
- Hits           4301     2107     -2194     
+ Misses         1227     1077      -150     
+ Partials         89       79       -10     
Impacted Files Coverage Δ Complexity Δ
...oud/aiplatform/v1beta1/utility/ValueConverter.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...form/v1beta1/stub/EndpointServiceStubSettings.java 79.86% <0.00%> (-9.92%) 20.00% <0.00%> (-1.00%)
...tform/v1beta1/stub/DatasetServiceStubSettings.java 78.49% <0.00%> (-9.35%) 23.00% <0.00%> (-1.00%)
...1beta1/stub/SpecialistPoolServiceStubSettings.java 79.36% <0.00%> (-9.05%) 17.00% <0.00%> (-1.00%)
...form/v1beta1/stub/PipelineServiceStubSettings.java 78.94% <0.00%> (-8.21%) 15.00% <0.00%> (-1.00%)
...rm/v1beta1/stub/PredictionServiceStubSettings.java 79.48% <0.00%> (-8.10%) 11.00% <0.00%> (ø%)
...latform/v1beta1/stub/ModelServiceStubSettings.java 78.33% <0.00%> (-8.00%) 22.00% <0.00%> (-1.00%)
...latform/v1beta1/stub/GrpcMigrationServiceStub.java 77.50% <0.00%> (-7.44%) 9.00% <0.00%> (-1.00%)
...rm/v1beta1/stub/GrpcSpecialistPoolServiceStub.java 83.82% <0.00%> (-7.22%) 12.00% <0.00%> (-1.00%)
...orm/v1beta1/stub/MigrationServiceStubSettings.java 78.12% <0.00%> (-7.01%) 12.00% <0.00%> (-1.00%)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0145cd...980e089. Read the comment docs.

@telpirion telpirion marked this pull request as ready for review December 17, 2020 03:59
@telpirion telpirion requested a review from a team December 17, 2020 03:59
@telpirion telpirion requested review from a team as code owners December 17, 2020 03:59
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • samples/snippets/pom.xml

Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Can we add a test for the bad input case as well?

* limitations under the License.
*/

package com.google.cloud.aiplatform.utility;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: most other utility packages use util

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 74 to 78
if (key.contains("multiLabel")) {
actualBoolValueEntry = entry;
} else if (key.contains("modelType")) {
actualStringValueEntry = entry;
} else if (key.contains("budgetMilliNodeHours")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should these checks for key name use equals?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}

Value actualBoolValue = (Value) actualBoolValueEntry.getValue();
Assert.assertEquals(testObjectInputs.getMultiLabel(), actualBoolValue.getBoolValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: usually Assert.assertEquals is statically imported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

private static AutoMlImageClassificationInputs testObjectInputs;

@Before
public void setUp() throws InvalidProtocolBufferException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything in this setup is only used in the one test - you can move it inside there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

…orm/util/ValueConverterTest.java

Co-authored-by: yoshi-code-bot <70984784+yoshi-code-bot@users.noreply.github.com>
@telpirion telpirion merged commit cf0b763 into master Dec 18, 2020
@telpirion telpirion deleted the enhanced-lib2 branch December 18, 2020 00:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
4 participants