Skip to content

Conversation

@winne42
Copy link
Contributor

@winne42 winne42 commented Jan 2, 2026

  • add missing test for TBool
  • as Java 11 is minimum now, List.copyOf() can be used

@google-cla
Copy link

google-cla bot commented Jan 2, 2026

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.

@winne42 winne42 force-pushed the feature/add-TBoolTest branch from 966fb69 to 0e67a8b Compare January 2, 2026 12:17
@winne42
Copy link
Contributor Author

winne42 commented Jan 2, 2026

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

fixed

Copy link
Collaborator

@Craigacp Craigacp left a comment

Choose a reason for hiding this comment

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

Thanks, I left a few comments with small changes.

@@ -0,0 +1,142 @@
package org.tensorflow.types;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add the same copyright statement as the other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, added

import org.tensorflow.op.math.LogicalNot;
import org.tensorflow.op.math.LogicalOr;

class TBoolTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be public to match the rest of the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this (in a separate commit) - but note that this is considered a code smell by SonarQube, cf. https://next.sonarqube.com/sonarqube/coding_rules?open=java%3AS5786&rule_key=java%3AS5786 or https://rules.sonarsource.com/java/quickfix/RSPEC-5786

.scalars()
.forEach(scalar -> assertEquals(false, scalar.getObject()));

// Calculate x AND y and validate the result
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be OR not AND.

Copy link
Contributor Author

@winne42 winne42 Jan 4, 2026

Choose a reason for hiding this comment

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

oops, changed

@winne42 winne42 force-pushed the feature/add-TBoolTest branch from 6851a69 to b87bfb2 Compare January 4, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants