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

ENH: Load combined scale and translation transforms to generated PVs #139

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

sbelsk
Copy link
Contributor

@sbelsk sbelsk commented Nov 5, 2024

Closes #133

To reduce clutter for the app users, after the generation of partial volumes in the pre-processing module, only the combined scale and translation transformation layer is loaded and applied onto each PV, instead of the two separate scale and translation trasnforms. I.e., {}.tfm is loaded and applied instead of {}_t.ftm and {}_scale.tfm.

All files are still written to the output directory as part of the PV generation proceess as before.

@sbelsk sbelsk requested a review from amymmorton November 11, 2024 15:36
Copy link
Contributor

@amymmorton amymmorton left a comment

Choose a reason for hiding this comment

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

image

Looks great
Tested with BN00124

Copy link
Contributor

@jcfr jcfr left a comment

Choose a reason for hiding this comment

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

This will definitely be helpful to reduce the "clutter" and help the user work with the set of loaded transform 💯

After addressing some minor nitpicks, this will be good for integration 🙏

AutoscoperM/AutoscoperM.py Outdated Show resolved Hide resolved
AutoscoperM/AutoscoperM.py Outdated Show resolved Hide resolved
AutoscoperM/AutoscoperM.py Show resolved Hide resolved
AutoscoperM/AutoscoperM.py Outdated Show resolved Hide resolved
@sbelsk sbelsk force-pushed the declutter-loaded-tfms-for-pvs branch 2 times, most recently from 4fafd67 to b778159 Compare November 12, 2024 14:59
@sbelsk sbelsk requested a review from jcfr November 18, 2024 13:38
To reduce clutter for the app users, after the generation of partial
volumes in the pre-processing module, only the combined scale and
translation transformation layer is loaded and applied onto each PV,
instead of the two separate scale and translation trasnforms. I.e.,
`{}.tfm` is loaded and applied instead of `{}_t.ftm` and `{}_scale.tfm`.

All files are still written to the output directory as part of the PV
generation proceess as before.

Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
@jcfr jcfr force-pushed the declutter-loaded-tfms-for-pvs branch from b778159 to 073925a Compare November 20, 2024 00:47
@jcfr jcfr merged commit 9792ad0 into BrownBiomechanics:main Nov 20, 2024
2 checks passed
@sbelsk sbelsk deleted the declutter-loaded-tfms-for-pvs branch November 20, 2024 15:48
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.

Load single bone.tfm instead of scale and translation separate
3 participants