Skip to content

Fixed choosing orientation for some previous INVALID conditions with greedy algorithm.#12

Merged
blowekamp merged 3 commits intoInsightSoftwareConsortium:masterfrom
blowekamp:FixOrientationSelection
Apr 23, 2021
Merged

Fixed choosing orientation for some previous INVALID conditions with greedy algorithm.#12
blowekamp merged 3 commits intoInsightSoftwareConsortium:masterfrom
blowekamp:FixOrientationSelection

Conversation

@blowekamp
Copy link
Copy Markdown
Member

@blowekamp blowekamp requested review from dzenanz and zivy April 22, 2021 20:29
@blowekamp
Copy link
Copy Markdown
Member Author

The same issue exits in itk::OrientImageFilter.

Comment thread test/itkDICOMOrientImageFilterGTest.cxx
@@ -170,27 +170,50 @@ DICOMOrientation::DirectionCosinesToOrientation(const DirectionType & dir)
// but it is DIFFERENT in the meaning of direction in terms of sign-ness.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

in commit message closes -> closest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dominate -> dominant

Comment thread src/itkDICOMOrientation.cxx Outdated
switch (max_c)
{
case 0: {
// When the dominate axis sign is positive, assign the coordinate for the direction we are increasing towards.
Copy link
Copy Markdown
Member

@dzenanz dzenanz Apr 22, 2021

Choose a reason for hiding this comment

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

Is dominate an adjective? If not, change it to dominant (same as in commit message).

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Apr 22, 2021

The same issue exits in itk::OrientImageFilter.

Can you update it in the same way?

The new method resolved the cases where there are no dominant
direction in one or more axis. This method chooses the high value in
the matrix first, then removes the invalid remaining values to select
from.
@blowekamp blowekamp force-pushed the FixOrientationSelection branch from 78a0b8e to 432d457 Compare April 23, 2021 17:44
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

In second commit's title closes -> closest.

@blowekamp
Copy link
Copy Markdown
Member Author

Thanks for the reviews :)

@blowekamp blowekamp merged commit 73ecf38 into InsightSoftwareConsortium:master Apr 23, 2021
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.

3 participants