feat: add support for dataset.default_rounding_mode by Gaurang033 · Pull Request #1688 · googleapis/python-bigquery

Gaurang033 · 2023-10-18T18:34:06Z

google-cla · 2023-10-18T18:34:11Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Linchin · 2023-10-19T19:45:42Z

google/cloud/bigquery/dataset.py

+        """
+        optional[str]: it could have one of the following possible value.
+        ROUNDING_MODE_UNSPECIFIED: Unspecified will default to using ROUND_HALF_AWAY_FROM_ZERO.
+        ROUND_HALF_AWAY_FROM_ZERO: ROUND_HALF_AWAY_FROM_ZERO rounds half values away from zero when applying precision
+                                    and scale upon writing of NUMERIC and BIGNUMERIC values.
+                                    For Scale: 0 1.1, 1.2, 1.3, 1.4 => 1 1.5, 1.6, 1.7, 1.8, 1.9 => 2
+        ROUND_HALF_EVEN: ROUND_HALF_EVEN rounds half values to the nearest even value when applying precision and scale
+                        upon writing of NUMERIC and BIGNUMERIC values.
+                        For Scale: 0 1.1, 1.2, 1.3, 1.4 => 1 1.5 => 2 1.6, 1.7, 1.8, 1.9 => 2 2.5 => 2
+        """


Thank you for adding the docstring. I think its format is not passing the docs test, see testing logs for more detailed information. Here is an example for docstring format.

done. i fixed it and run nox -s docs without any error.

Linchin · 2023-10-19T19:48:39Z

google/cloud/bigquery/dataset.py

+        """
+        return self._properties.get("defaultRoundingMode")
+
+    @default_rounding_mode.setter


The test coverage is not passing because the setter function is not covered. Could you please also add unit test for this? Thank you!

@Linchin should I modify this test_create_dataset_w_attrs ?

I think it would be better to add several smaller tests (in parallel to test_create_dataset_with_default_rounding_mode() as you added), each for a case in our if conditions. These include:

value is None

value is not a string

value not in possible_values

value is in possible_values

Linchin · 2023-10-19T19:51:21Z

tests/system/test_client.py

+        if kwargs.get("location"):
+            dataset.location = kwargs.get("location")
+        if kwargs.get("max_time_travel_hours"):
+            dataset.max_time_travel_hours = kwargs.get("max_time_travel_hours")


I see you have also opened #1683 that covers max_time_travel_hours. Let's make these changes over there, and confine this PR to only default_rounding_mode stuff.

@Linchin done. cherry pick the commit to that pull request.

Could you also delete these lines in this PR? Thank you!

Linchin · 2023-10-19T19:52:54Z

@Gaurang033 Thank you for your contribution! The PR looks good except for a few comments that need to be addressed. Please let us know if you need any help with them :)

Linchin · 2023-10-20T19:19:38Z

google/cloud/bigquery/dataset.py

+
+        Raises:
+            ValueError: for invalid value types.
+


Suggested change

@Linchin sorry. what change should i make ?

Just remove this line

@Linchin done. made the changes. could you please review again and let me know if I need to make any other changes.

Linchin · 2023-10-27T21:54:00Z

There's a mypy test error that's blocking this PR. Created #1705 to fix this.

Co-authored-by: Lingqing Gan <lingqing.gan@gmail.com>

Gaurang033 requested a review from a team October 18, 2023 18:34

Gaurang033 requested a review from a team as a code owner October 18, 2023 18:34

Gaurang033 requested a review from jainsahab October 18, 2023 18:34

product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery API. labels Oct 18, 2023

Gaurang033 force-pushed the feature/default_rounding_mode branch from 590d65e to cf723d9 Compare October 18, 2023 18:36

Linchin requested review from Linchin and chalmerlowe and removed request for jainsahab October 19, 2023 18:45

Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023

Linchin reviewed Oct 19, 2023

View reviewed changes

Gaurang033 force-pushed the feature/default_rounding_mode branch from dd54292 to 03becfe Compare October 20, 2023 16:35

Linchin reviewed Oct 20, 2023

View reviewed changes

Gaurang033 force-pushed the feature/default_rounding_mode branch 3 times, most recently from 3c57278 to 5d43d87 Compare October 20, 2023 23:40

Gaurang033 requested a review from Linchin October 23, 2023 13:08

feat: add support for dataset.default_rounding_mode

b063e8a

Gaurang033 force-pushed the feature/default_rounding_mode branch from 5d43d87 to b063e8a Compare October 24, 2023 01:22

Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2023

Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2023

Merge branch 'main' into feature/default_rounding_mode

8ef7389

Linchin approved these changes Oct 27, 2023

View reviewed changes

Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2023

Linchin enabled auto-merge (squash) October 28, 2023 06:04

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2023

Linchin merged commit 83bc768 into googleapis:main Oct 28, 2023

release-please bot mentioned this pull request Oct 28, 2023

chore(main): release 3.13.0 #1676

Merged

Gaurang033 deleted the feature/default_rounding_mode branch October 29, 2023 22:49

Linchin added a commit to Linchin/python-bigquery that referenced this pull request Oct 30, 2023

feat: add support for dataset.default_rounding_mode (googleapis#1688)

011f829

Co-authored-by: Lingqing Gan <lingqing.gan@gmail.com>

This was referenced Nov 7, 2023

November 06, 2023 kitta65/bq-extension-vscode#251

Closed

November 06, 2023 kitta65/prettier-plugin-bq#258

Closed

November 06, 2023 kitta65/bq2cst#268

Closed

Conversation

Gaurang033 commented Oct 18, 2023

Uh oh!

google-cla bot commented Oct 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Linchin Oct 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Linchin commented Oct 19, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Linchin commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Linchin Oct 20, 2023 •

edited

Loading