search

PTB-MR / mrpro

MR image reconstruction and processing.
https://ptb-mr.github.io/mrpro/
Apache License 2.0
9 stars 3 forks source link

Typos+Formatting #192

Open koflera opened 4 months ago

koflera commented 4 months ago

A collection of typos + formatting issues still present in the main branch which could address in a separate PR once the list becomes long enough...

koflera commented 4 months ago

FIXED

ckolbPTB commented 4 months ago

parameter, inital https://github.com/PTB-MR/mrpro/blob/ccc19689bec9ad3c43ad85b3ff3ed6de37b5204e/tests/algorithms/test_optimizers.py#L82

FIXED

koflera commented 4 months ago

paramters should parameters, inital should be initial before should begrad_before https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/tests/algorithms/test_optimizers.py#L82

FIXED

ckolbPTB commented 4 months ago

WASABI and WASABITI signal model classes should be defined as _WASABI and _WASABITI and then they should be imported via init here https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/src/mrpro/operators/models/__init__.py#L3

FIXED in #218

ckolbPTB commented 4 months ago

If c is coils then use coils rather than c in

https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/src/mrpro/operators/models/_WASABITI.py#L92C9-L92C60

and

https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/src/mrpro/operators/models/_WASABI.py#L87

remove print statement https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/src/mrpro/operators/models/_WASABITI.py#L91

FIXED

ckolbPTB commented 4 months ago

Imports in tests should be from mrpro.operators.models import InversionRecovery rather than from mrpro.operators.models._InversionRecovery import InversionRecovery

https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/tests/operators/models/test_t1_models.py#L4 https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/tests/operators/models/test_wasabi.py#L4 https://github.com/PTB-MR/mrpro/blob/250d0ad528406274c4b9b5f0a3f1d687f325aa55/tests/operators/models/test_wasabiti.py#L4

FIXED in #218

schuenke commented 4 months ago

In the latest commit f7c9ccb to #193 I enabled the typos pre-commit hook. The tool found and fixed all mentioned typos as far as I can tell.

IMO it's nicer to have an automated typo detection and correction than manually reporting them here.

Still, the "wrong" imports of the signal models etc. have to be corrected manually...

ckolbPTB commented 4 months ago

The tool found and fixed all mentioned typos as far as I can tell.

That's great!

I guess the tool will "only" fix typos in comments (which is a good thing) and there might still be typos in variable/function/class names which we could collect here.

ckolbPTB commented 2 weeks ago

"if" instead of "is"

https://github.com/PTB-MR/mrpro/blob/5fa11c0991e013d32faa0f4a1dfdeef1aa880af3/src/mrpro/data/AcqInfo.py#L104

Another typo https://github.com/PTB-MR/mrpro/blob/0730e0608366b422c59c0c5ffa50a94fba671cf2/src/mrpro/operators/FastFourierOp.py#L105

ckolbPTB commented 2 weeks ago

we use max_iters and max_iterations in our code but I think it should be n_max_iterations everywhere

superseded by #340

ckolbPTB commented 2 weeks ago

should we follow e.g. official numpydoc style for references: https://numpydoc.readthedocs.io/en/latest/format.html#references as we did here https://github.com/PTB-MR/mrpro/blob/0730e0608366b422c59c0c5ffa50a94fba671cf2/src/mrpro/utils/Rotation.py#L640

ckolbPTB commented 2 weeks ago

Sometimes we use Raises and Returns in the docstring sometimes we don't.

JoHa0811 commented 11 hours ago

should we follow e.g. official numpydoc style for references: https://numpydoc.readthedocs.io/en/latest/format.html#references as we did here

https://github.com/PTB-MR/mrpro/blob/0730e0608366b422c59c0c5ffa50a94fba671cf2/src/mrpro/utils/Rotation.py#L640

Pulled this into its own seperate issue #351 to link the according PR