Add error message for impossible roi request to ZeissDataReader and fix sign error when negative values passed to ROI

[lauramurgatroyd]

Description

• If the ROI bound is larger than the axis, a ValueError will be raised. Previously, it would go on to crop the geometry but not the data and would throw an error when trying to create the AcquisitionData to return, which was not clear to the user what was wrong.
• If a negative number is given to the ROI, previously this was subtracted (so double negative) whereas it should've been added (single negative) to the total axis length.

Closes #2240

Example Usage

Contribution Notes

• The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in CIL (the Work) under the terms and conditions of the Apache-2.0 License
• I confirm that the contribution does not violate any intellectual property rights of third parties

Changes

Testing you performed

Please add any demo scripts to https://github.com/TomographicImaging/CIL-Demos/tree/main/misc

Related issues/links

Checklist

• I have performed a self-review of my code
• I have added docstrings in line with the guidance in the developer guide
• I have updated the relevant documentation
• I have implemented unit tests that cover any new or modified functionality
• CHANGELOG.md has been updated with any functionality change
• Request review from all relevant developers

[gfardell]

Are there any unit tests that need updating here?

[gfardell]

The stop could be a negative, i.e. -10 would be the N-10. Would these checks pass? has the value already been normalised?

[lauramurgatroyd]

currently its only dealing with the stop being negative. I'm now updating it to allow a negative start as well

[lauramurgatroyd]

Just updated to make sure it can take negative start and stop values, plus added unit tests for these cases

[hrobarts]

Hi Laura, this looks good to me. Is it worth adding something about the negative indexing in the docstring?

[lauramurgatroyd]

Suggested change

	For ImageData to skip files or to change number of files to load,
	adjust ``vertical``. E.g. ``'vertical': (100, 300)`` will skip first 100 files
	and will load 200 files.
	``'axis_label': -1`` is a shortcut to load all elements along axis.
	``start`` and ``end`` can be specified as ``None`` which is equivalent
	to ``start = 0`` and ``end = load everything to the end``, respectively.
	``start`` and ``end`` can be negative numbers, with the same behaviour
	as numpy slicing, with -x meaning x many from the end.
	e.g on a dimension with 10 elements ``start=-3`` and ``stop=-1``
	is equivalent to ``start=7``, ``stop=9``

[lauramurgatroyd]

Hi @hrobarts sorry the suggestion looks weird but is line 53 onwards sufficient for adding the details of negative values in the roi?

[gfardell]

I think you should keep it simple. We use it all over the place (maybe check slicer/binner doc strings for consistency). I would think just saying it accepts negative indices is sufficient.

The main point is the start index is included in the range, the stop index won't be included in the range, I've seen people put (0,-1) not realising that is cropping, whereas (100,-100) will be a symmetric crop.

I think it's easier to convey in a code example snippet than text though.

If you want to explain the indexing maybe refer to: https://numpy.org/doc/stable/user/basics.indexing.html but I would class it as a language feature.