Draft: Short review

Hello y'all,

as announced I had a look into the package and wanted to give some feedback. Overall the project looks quite good but there are some choices I dont understand or would do differently. I will try to give some constructive feedback and hope you can take something from it. Ignore it if you dont agree :D

General stuff

• Why do you use a precommit.sh file instead of the python pre-commit framework? I think it would be easier to maintain and more user friendly. See here https://pre-commit.com/ and ruff specific here https://github.com/astral-sh/ruff-pre-commit
• Is there a reason for not using pypi? The hotopy package is not reserved yet and I think it would be easier for users to install without defining the extra-index-url. Also you could use the same CI/CD of gitlab to automatically deploy the package to pypi.
• I would add matrix to the CICD to run it against different python versions (atleast 3.9-3.12). A classifier for the supported python version should also be present in the pyproject.toml
• I like that you have some tests, but they are not run in the CI/CD and also outdated? For me they did not run successfully. I would suggest to use pytest and to run the tests in the CI/CD. Also I would suggest to use a coverage tool like coverage or pytest-cov to check the coverage of your tests. I know, lots of work but it is worth it in the long run imo.

Regarding docs

• Why are the examples not placed in this repository? Is there a reason for placing them in another repo? For my projects I normally use myst-nb which allows among others to write docs in markdown and to include ipynb files. See here https://myst-nb.readthedocs.io/. If you want I can help you with the setup.
• I think the docs might need a quickstart guide and maybe a guide or two for each different subpackage (tomo, holo...). Or at least the most common use cases as notebooks. I think the current docs are a bit sparse in that regard.
• I would personally move the contribution guide to the docs as well. You can always link to it in the readme.
• I think adding intrasphinx links would be helpful. You can reference other classes, functions, etc. in the docs. E.g. linking to numpy could be useful.

Code style and structure

I noticed that sometimes your typehints aren't correctly set. Maybe your editor is not configured correctly? I noticed the following issues:

• None is often not defined as a possible value. For instance in optimize.py you have state:dict = None but it should be state:dict|None = None, this union with None is missing in many places.
• You may use the__from __future__ import annotations to avoid the need of the quotation marks in the typehints. E.g. monitor:"Monitor" = None would be monitor:Monitor|None = None if you use the future import.
• Generally alot of np.array typehints are missing, even tho np typehints are not that useful in general I think there is some value in adding them for finding some undefined behavior. You can find the overview for np typehints here https://numpy.org/devdocs/reference/typing.html. For example the hlcc function call could look like

def hlcc(g: npt.NDArray, k: npt.NDArray[np.int64] | int, L: float = 1.0) -> npt.NDArray:
    pass

• If a class should not be used by itself you can mark it as an abstract base class or use a protocol. I think e.g.Algorithm, _PadderBase, AstraWrapper should be an abstract base class

If you want to enforce typehints a bit more you can setup a mypy or beartype flow in the CI/CD.

Things already fixed in this PR

• For io filenames you use should use Path | str as a typehint. And use paths if possible.
• _PadderBase is now an abstract base class mainly as reference if you intend to update the other code

---

While there are some areas for improvement, I hope my feedback is useful and has some actionable suggestions. With a few refinements, the toolbox could become even more user-friendly, and maintainable.

Keep up the great work, and I look forward to seeing how the project evolves! 🚀

Best regards, Sebastian