Currently only ImageData is covered with tests not QMRIData.

We have default docstrings in some places,
for example

should contain a description and the units etc in the docstrings, instead of
_description_

Somebody should go through main and see where we messed up so far and fix it, and we should maybe remember to check in in code review (Mea culpa, I missed it).

On the other work-in-progress branches we might have also those, I will try to remember to complain about those during the review of the PRs..

Hey everbody,

close to the end, CK and I had the discussion about some of the None's

I believe we came to the following conclusions:

• We should avoid returning Nones (and typing everything like it can return Nones)
• QMRIData should not return None's if asked for some map.
  • either it returns a default map or it throws an exception if the values has not been
    set and a default map is not possible.
• An ImageData without data should maybe not exist, i.e. force setting the image in the constructor. This way, the numpy conversion never has to return None as well.

Otherwise we have to check for None everywhere and write lots tests that handle these corner cases in each downstream function...

What do you think?

This line should fail if the qmri shape checks work as expected.

The qmrdata object should get a shape fixed at the first assignment of .t1 . the second assignment with a different shape should then fail.

we should fix both.

If I undestand it correctly than currently we don't specify anywhere if we want a tensor on the CPU or GPU.

Would it make sense that the MRData class has a function .cuda() and .cpu()? This would then e.g. move _data and _mask to GPU/CPU for ImageData and _t1 _rho and _mask for QMRIData.

How do you normally specify the device? Is this a global variable you set and then everything is put to the same device or do you specify this for each tensor?

https://github.com/ckolbPTB/InhomCorr/blob/3dfc29a6d460eabf027778ecad8dab9248036fb4/.github/workflows/codecov.yml#LL22C30-L22C30

Is the most likelz culprit. This should be a pip install -e .

We discussed and decided to move the base classes/interfaces/... from the "interface" subfolder to the individual submodules and adjust the naming accroding to what they actually are.

MRData
QMRData
ImageData

should be moved to the inhomcorr folder (not into a specific submodule)

Like Python has built-in support for properties, it also has built-in functionality for indexable objects via getitem .
An example would be the get_data function in for the dataloaders. We could try to design it in a way that the QMRIDataLoader is indexable (Pseudo-Code):

class QMRIDataLoader:
     def __getitem__(self, id):
          return self.item[0]

Then, later one, we could use it as

obj=QMRIDataLoader(...)
obj[0]

This would make it more 'pythonic' and consistent, for example, with how pytorch datasets are supposed to work.

This will be a reminder for missing tests.

So far we have not specified the unites of e.g. t1 anywhere. We could

1. add this in the comments of QMRIData
2. change "t1" to "t1_in_s"

I would prefer option 2. What do you think?

I believe it would be usefuull to just be able to set the image in the constructor. There are lots of places were we otherwise have the sequence of

img=ImageData()
img.data=image

I would even go so far to have the image mandatory in the init. Soi we never have an imageData that does not have an image tensor.

The path of where the tests are installed might not be writable when executing the tests.