Typing NDCube

[samaloney]

PR Description

Add types and type checking to NDCube

• Some of the types could probably use astropy's QuantityLike
• Some of the types weren't clear so had to guess
• Found some real or at least potential bugs (commit e1808f4)

AI Assistance Disclosure

AI tools were used for:

• [x ] Code generation (e.g., when writing an implementation or fixing a bug)
• [ ] Test/benchmark generation
• [ ] Documentation (including examples)
• [x ] Research and understanding
• [ ] No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

[samaloney]

Not sure how to resolve the doc build issue I think its because there is more then one slice object so it can be uniquely resolved?

[DanRyanIrish]

More comments to follow

[DanRyanIrish]

I don't think we've decided that resample should live in the ABC. My initial reaction is that I'm not against it. But we shouldn't add this without making an active decision.

[samaloney]

Ok yea can pull it from this PR and update the type hints if needed

[DanRyanIrish]

Can you explain what this test is doing? Is doesn't appear to be testing slicing as its comments suggest.

[samaloney]

Yea so unaligned_collection looks like this and then it sliced with unaligned_collection[("cube0", "cube1")]

<ndcube.ndcollection.NDCollection object at 0x10ffbc4d0>
NDCollection
------------
Cube keys: ('cube0', 'cube1', 'cube2')
Number of Cubes: 3
Aligned dimensions: None
Aligned physical types: None

[DanRyanIrish]

Should key_data_pairs not be Iterable[Any, Any]? Or whatever the correct equivalent is?

[samaloney]

Yep so this should accept a dict of type dict[str, NDCube|NDSequence] or tuple on the tuple is is a tuple of tuple e.g tuple[tuple[str, NDCube|NDSequence] or a long list of key, data pair e.g tuple[any, ...]. I see if I can worked it out from the code.

[DanRyanIrish]

Why is this needed? keys is defined a couple line below. And key_data_pairs must contain at least one valid key so keys cannot be empty. Seems to me like it isn't needed. But am I missing something?

[samaloney]

So this is because pyright raises and possibly unbound warning because aligned_axes is None the variable isn't defined - could add local comment disable that warning

[DanRyanIrish]

This should return None or a dict where the value assigned to each key is int or tuple[int]. Can this be captured in typing here?

[samaloney]

Yea I think dict[Any, int] | dict[Any, tuple[int, ...]] | None`

Also where typing could potentially help design - as consumer of this I need to check if the value is an int or tuple then maybe the length of the tuple but if it was always returned a tuple (), (1,), (1, 2, 3) can always use len` - not suggestion this should be changed by the way just a thought.

[samaloney]

So if I change the type annotation to it raises an additional type errors as in the current code later on this properties value is subscripted so if were an int this would raise an error.

ndcube/ndcube/ndcollection.py

Lines 229 to 255 in 9befb9b

	# Case 1: int
	# First aligned axis is dropped.
	if isinstance(item, int):
	drop_aligned_axes_indices = [0]
	# Insert item to each cube's slice item.
	for i, key in enumerate(self):
	collection_items[i][self.aligned_axes[key][0]] = item
	# Case 2: slice
	elif isinstance(item, slice):
	# Insert item to each cube's slice item.
	for i, key in enumerate(self):
	collection_items[i][self.aligned_axes[key][0]] = item
	# Note that slice interval's of 1 result in an axis of length 1.
	# The axis is not dropped.
	# Case 3: tuple of ints/slices
	# Search sub-items within tuple for ints or 1-interval slices.
	elif isinstance(item, tuple):
	# Ensure item is not longer than number of aligned axes
	if len(item) > self.n_aligned_axes:
	raise IndexError("Too many indices")
	for i, axis_item in enumerate(item):
	if isinstance(axis_item, int):
	drop_aligned_axes_indices.append(i)
	for j, key in enumerate(self):
	collection_items[j][self.aligned_axes[key][i]] = axis_item

Internally its always converted to a tuple the only place I can see where it can be an int is in _sanitize_user_aligned_axes also tuple(int, ...) cover