Skip to content
This repository was archived by the owner on Jul 15, 2025. It is now read-only.

Kotlin friendly names#1

Merged
karllessard merged 6 commits into
tensorflow:mainfrom
rnett:rn_kotlin_friendly_names
May 18, 2021
Merged

Kotlin friendly names#1
karllessard merged 6 commits into
tensorflow:mainfrom
rnett:rn_kotlin_friendly_names

Conversation

@rnett

@rnett rnett commented May 15, 2021

Copy link
Copy Markdown
Contributor

Renames Shape.size to Shape.get and add a toListOrNull method. Was part of tensorflow/java#165.

@Craigacp Craigacp 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.

Could you provide some rational for why the method is renamed? Does kotlin treat get methods specially, or is there some conflict with size?

Comment thread ndarray/src/main/java/org/tensorflow/ndarray/Shape.java
Comment thread ndarray/src/main/java/org/tensorflow/ndarray/StdArrays.java
Comment thread ndarray/src/main/java/org/tensorflow/ndarray/Shape.java Outdated
Comment thread ndarray/src/main/java/org/tensorflow/ndarray/Shape.java Outdated
@rnett

rnett commented May 17, 2021

Copy link
Copy Markdown
Contributor Author

Yeah, Kotlin will automatically treat get as an operator, and rather than adding an extension in the Kotlin API Karl suggested I just change it here. Avoids overloading the meaning of size, too.

Comment thread ndarray/src/main/java/org/tensorflow/ndarray/StdArrays.java Outdated

@Craigacp Craigacp 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.

I think I'm fine with this, but I'll let @karllessard merge it in case he has comments.

Comment thread ndarray/src/main/java/org/tensorflow/ndarray/Shape.java
@rnett rnett requested a review from karllessard May 18, 2021 20:43
@karllessard karllessard merged commit d2aed4a into tensorflow:main May 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants