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

Adapt KHeader parameters #425

Closed
wants to merge 278 commits into from
Closed

Adapt KHeader parameters #425

wants to merge 278 commits into from

Conversation

ckolbPTB
Copy link
Collaborator

@ckolbPTB ckolbPTB commented Oct 1, 2024

No description provided.

ckolbPTB and others added 30 commits August 25, 2023 21:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Make it clear that it is only a workaround
to be used until we have the first real trajectories.
Otherwise, e.g, torch.Tensor will be considered as Any.
It seems not all of the clear spelling issues are clear spelling issues?

Leave this open for further discussion.
* ismrmrd test data class added, pytest fixture to create data only once

* phantom with ellipses added for testing but also development

* flake8 does not check test folder during pre-commit

---------

Co-authored-by: Patrick Schuenke <[email protected]>
@fzimmermann89
Copy link
Member

Maybe we can discuss UserParameters again I we make misc more private and unsupported

trajectory_dimensions=tensor_2d(headers['trajectory_dimensions']).fill_(3), # see above
user_float=tensor_2d(headers['user_float']),
user_int=tensor_2d(headers['user_int']),
version=tensor_2d(headers['version']),
)
return acq_info

def _apply_(self, modify_acq_info_field: Callable) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense it instead use the apply_ from the movedatamixin, as it should do the same, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but I think we need #489 first, don't we?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.. I wrote that in slack.. give me a bit of time to order things tonight and then merge them in a sane order

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently figuring out for me what a good order could be

ghstack-source-id: ece2c18bcaf513233da227f1cd8640fdae3b1b29
ghstack-comment-id: 2441645633
Pull Request resolved: #489
@fzimmermann89 fzimmermann89 changed the base branch from main to apply November 10, 2024 02:09
@fzimmermann89 fzimmermann89 added the pre-commit.ci autofix run autofix in this PR label Nov 10, 2024
@pre-commit-ci pre-commit-ci bot removed the pre-commit.ci autofix run autofix in this PR label Nov 10, 2024
fzimmermann89 and others added 5 commits November 10, 2024 15:47
commit 807856c
Author: Felix F Zimmermann <[email protected]>
Date:   Sun Nov 10 03:09:05 2024 +0100

    move to apply_ of dataclasses

commit 83e98a8
Author: ckolbPTB <[email protected]>
Date:   Fri Oct 25 17:25:51 2024 +0200

    slice, phase, read dir have to form proper rotation in tests

commit b2f042e
Author: ckolbPTB <[email protected]>
Date:   Fri Oct 25 17:08:23 2024 +0200

    doc string for misc

commit c5d9cec
Author: ckolbPTB <[email protected]>
Date:   Fri Oct 25 17:05:30 2024 +0200

    review

commit c713e37
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 24 21:18:24 2024 +0200

    fix rotation

commit 101d682
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 24 21:11:55 2024 +0200

    rotation

commit dc9de17
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 24 20:52:55 2024 +0200

    adapt acq_info to new get_item set_item syntax

commit 6852aac
Merge: 7155a87 eecdf49
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 24 13:58:55 2024 +0200

    merge main

commit 7155a87
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 3 17:50:47 2024 +0200

    sort_idx added

commit b7dc10f
Author: ckolbPTB <[email protected]>
Date:   Thu Oct 3 16:57:07 2024 +0200

    quaternion for siemens is phase read slice

commit 539acff
Author: ckolbPTB <[email protected]>
Date:   Wed Oct 2 16:58:15 2024 +0200

    adapt _apply_ in kdata

commit c3d4187
Author: ckolbPTB <[email protected]>
Date:   Wed Oct 2 16:51:04 2024 +0200

    _apply_ instead of modify_acq_info

commit 809b68d
Merge: 9c3d61e 763ce96
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 20:36:12 2024 +0200

    Merge branch 'main' into unit_conversion_utils

commit 9c3d61e
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 16:22:22 2024 +0200

    trajectory description removed

commit ccd93f7
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 16:17:34 2024 +0200

    orientation as rotation

commit 4c4b7fa
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 13:36:19 2024 +0200

    n_coils removed

commit bf722c2
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 11:56:09 2024 +0200

    kheader clean-up

commit 27eba47
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 11:38:42 2024 +0200

    unit conversion finished

commit 4a4f727
Merge: daf40fa f11f047
Author: ckolbPTB <[email protected]>
Date:   Tue Oct 1 08:57:05 2024 +0200

    conflict solved

commit daf40fa
Author: ckolbPTB <[email protected]>
Date:   Mon Sep 30 17:32:37 2024 +0200

    current tests moved
@fzimmermann89
Copy link
Member

I f'ed this up. sorry.

@ckolbPTB can you create a new PR from the unit_conversion_utils branch to main?

@fzimmermann89
Copy link
Member

New PR: #506

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 this pull request may close these issues.

None yet