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

Conversation

@karllessard
Copy link
Contributor

Just unlocks SparseNdArray implementations constructor to be extended by sub-classes (requires for TensorFlow sparse tensor support)

Copy link
Contributor

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

LGTM. The diffs seem bigger than they should be given you just made a method protected, but that doesn't really matter.

* element in indices. For example, given {@code indices=[[1,3], [2,4]]}, the parameter {@code
* values=[18, 3.6]} specifies that element {@code [1,3]} of the sparse NdArray has a value of
* {@code 18}, and element {@code [2,4]} of the NdArray has a value of {@code 3.6}.
* @param defaultValue Scalar value to set for indices not specified in {@link #getIndices()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be {@code indices} rather than {@link #getIndices()}, as it refers to the construction parameter rather than some instance method that you can't actually call when constructing the object.

@karllessard karllessard merged commit d10262e into tensorflow:main Jan 11, 2022
@karllessard karllessard deleted the sparse-parent branch January 11, 2022 15:34
@karllessard
Copy link
Contributor Author

LGTM. The diffs seem bigger than they should be given you just made a method protected, but that doesn't really matter.

That's because I've moved the now-protected constructor up to the top of the class, to respect the visibility ordering of the class members. Thanks Adam

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.

2 participants