Skip to content

adding api doc and cleanup of Shape and related concepts#94

Merged
marcelluethi merged 4 commits into
dimwit-dev:mainfrom
marcelluethi:api-doc-shape-axis
May 6, 2026
Merged

adding api doc and cleanup of Shape and related concepts#94
marcelluethi merged 4 commits into
dimwit-dev:mainfrom
marcelluethi:api-doc-shape-axis

Conversation

@marcelluethi
Copy link
Copy Markdown
Contributor

  • Add API doc to all classes
  • Reorder concepts in Axis
  • Add Label instance to axis
  • Make Axis a concrete class

- Add API doc to all classes
- Reorder concepts in Axis
- Add Label instance to axis
- Make Axis a concrete class
@marcelluethi marcelluethi requested a review from benikm91 April 21, 2026 11:36
* @param args A tuple of AxisExtents from which to create the shape.
* @return A shape with dimensions corresponding to the sizes of the AxisExtents in the
*/
def apply[A <: Tuple](args: A)(using ev: Union[A] <:< AxisExtent[?], n: Labels[ExtractLabels[A]]): Shape[ExtractLabels[A]] =
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is definition change intended? Seems like Union[A] evidence was added.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes that was by purpose. The idea was to make it clear that it needs to be a subtype of AxisExtent and not just an arbitrary tuple. Not sure if it helps - no strong opinion.

* @param args A tuple of AxisExtents from which to create the shape.
* @return A shape with dimensions corresponding to the sizes of the AxisExtents in the tuple.
*/
def fromTuple[A <: Tuple](args: A)(using ev: Union[A] <:< AxisExtent[?], n: Labels[ExtractLabels[A]]): Shape[ExtractLabels[A]] =
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See previous comment

case _ => jaxValue.toString()

def extent[L](axis: Axis[L])(using ev: AxisIndex[T, L]): AxisExtent[L] =
def extent[L: Label](axis: Axis[L])(using ev: AxisIndex[T, L]): AxisExtent[L] =
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was label evidence added on purpose. I suggest removing it if not necessary in the method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also here I thought that adding a label for documentation could help. It is not really needed but in most other method when we have this type parameter it also needs to be a label. so why not adding it here? Again, I have no strong opinion.

import scala.annotation.publicInBinary
import ShapeTypeHelpers.AxisIndex
import dimwit.tensor.{Labels, Label}
import scala.Tuple.Union
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See other comments

* @param axis The axis for which to retrieve the extent.
* @return The extent of the specified axis.
*/
def extent[L: Label](axis: Axis[L])(using ev: AxisIndex[T, L]): AxisExtent[L] = AxisExtent(axis, this(axis))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

L Label evidence added. See other comment.

@benikm91
Copy link
Copy Markdown
Collaborator

Please check comments. If changes are intended and make sense I will accept PR.

marcelluethi and others added 2 commits April 28, 2026 12:49
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
@marcelluethi
Copy link
Copy Markdown
Contributor Author

The issues have been resolved by using better names for the type parameters and arguments.

@benikm91 benikm91 self-requested a review May 1, 2026 14:54
@marcelluethi marcelluethi merged commit ade83e2 into dimwit-dev:main May 6, 2026
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.

2 participants