Conversation
…g into sptial_transforms_update
Codecov Report
@@ Coverage Diff @@
## master #38 +/- ##
=========================================
Coverage ? 94.58%
=========================================
Files ? 36
Lines ? 1423
Branches ? 0
=========================================
Hits ? 1346
Misses ? 77
Partials ? 0
Continue to review full report at Codecov.
|
justusschock
left a comment
justusschock
left a comment
There was a problem hiding this comment.
I got several questions but so far this seems to be promising.
One general question: I just realised that we often have the case that we need to check if something was already computed and if not we call a function to do the actual computation. Should we maybe think of something like a "lazy property"?
| return data | ||
|
|
||
| @abstractmethod | ||
| def augment_grid(self, grid: Dict[Tuple, Tensor]) -> Dict[Tuple, Tensor]: |
There was a problem hiding this comment.
What is this function for?
Maybe it would be better to have this return the grid per default (as done in the affines).
There was a problem hiding this comment.
This function gets a grid and applies its augmentation to it. It returns a grid for every spatial size which was found inside the keys
| return grid | ||
|
|
||
| def __add__(self, other): | ||
| if not isinstance(other, GridTransform): |
There was a problem hiding this comment.
Can't we also add the case, where there is just the grid provided and not wrapped with an transform?
| __all__ = ["GridTransform", "StackedGridTransform"] | ||
|
|
||
|
|
||
| class GridTransform(AbstractTransform): |
There was a problem hiding this comment.
What if I somehow get my grid from somewhere else and just want to use it? can we have an optional argument in here?
There was a problem hiding this comment.
Sorry, thought the upper comment was at the same position. ^^
Yeah, we can add that. I think I would prefer an extra Transform for that.
| if isinstance(transforms, (tuple, list)): | ||
| if isinstance(transforms[0], (tuple, list)): | ||
| transforms = transforms[0] | ||
| self.transforms = transforms |
There was a problem hiding this comment.
Can we also include a type check here which checks, that each item is a valid GridTransform?
2 participants
Short Description
Introduce Grid Transformations including:
Current limitation: Affine Transforms need to be performed before GridTransforms
This should be merged after #31
PR Checklist
PR Implementer
This is a small checklist for the implementation details of this PR.
If you submit a PR, please look at these points (don't worry about the
RisingTeamand
Reviewerworkflows, the only purpose of those is to have a compact view ofthe steps)
If there are any questions regarding code style or other conventions check out our
summary.
__all__sections and__init__RisingTeam
RisingTeam workflow
Please make sure to communicate the current status of the pr.)
closes #IssueNumberat the bottom ifnot already in description)
Reviewer
Reviewer workflow
risingdesign conventions?Can you think of critical points which should be covered in an additional test?