Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageData should take the image in the constructor #16

Closed
fzimmermann89 opened this issue Apr 26, 2023 · 4 comments · Fixed by #26 or #32
Closed

ImageData should take the image in the constructor #16

fzimmermann89 opened this issue Apr 26, 2023 · 4 comments · Fixed by #26 or #32
Assignees

Comments

@fzimmermann89
Copy link
Collaborator

def __init__(self) -> None:

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.

@ckolbPTB
Copy link
Owner

I agree or can anyone think of a use case where an empty ImageData object would be required?

@schuenke
Copy link
Collaborator

I also vote for including the image in the constructor. I cannot think of any case where we need an empty object. However, having the option to change the image data later makes sense imo.

@schuenke
Copy link
Collaborator

schuenke commented May 2, 2023

For ImageData we decided to enforce data in the constructor.
For QMRI:
def init(self, t1=None,rho=None, ...)
(some logic)
--> you can provide maps in the constructor. If you do, the shapes will be checked. If you don't, shape will be (1,1) implicitly.

@schote
Copy link
Collaborator

schote commented May 2, 2023

I agree or can anyone think of a use case where an empty ImageData object would be required?

Potentially, you could have an Image object which only contains the mask from the MRData class?

@schote schote linked a pull request May 2, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants