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

BUG: Remove obsolete VRG Generation functionality #140

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

sbelsk
Copy link
Contributor

@sbelsk sbelsk commented Nov 10, 2024

Closes #132.

This PR removes the 'Manual Camera Placement' and 'Automatic Camera Placement' VRG Generation UI groups from the Pre-Processing modules, along with all of their associated backend code. This includes the removal of the CalculateDataIntensityDensity and VirtualRadiographGeneration modules.
The support for camera placement is superseded by the hierarchical registration capability.

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.

Thanks for working on those changes 🙏

Beside of few minor nitpicks, it will be good for integration

In the commit message, could you also provide the rational for removing the removing the features ? (e.g superseded by the integration of hierarchical registration allowing to ...)

AutoscoperM/AutoscoperMLib/IO.py Outdated Show resolved Hide resolved
AutoscoperM/AutoscoperMLib/IO.py Show resolved Hide resolved
@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.

LGTM

image

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.

Thanks for addressing the comments 🙏

I suggest to further tweak the commit message and pull request description to include the rational. Feel free to tweak/adapt as you see fit.

Removes the 'Manual Camera Placement' and 'Automatic Camera Placement'
VRG Generation UI groups from the Pre-Processing modules, along with all
of their associated backend code. This includes the removal of the
CalculateDataIntensityDensity and VirtualRadiographGeneration modules.
+ The support for camera placement is superseded by the hierarchical registration capability.

@sbelsk sbelsk force-pushed the remove-VRG-generation branch from a7310b0 to 3ade5fc Compare November 18, 2024 16:24
Removes the 'Manual Camera Placement' and 'Automatic Camera Placement'
VRG Generation UI groups from the Pre-Processing modules, along with all
of their associated backend code. This includes the removal of the
CalculateDataIntensityDensity and VirtualRadiographGeneration modules.
The support for camera placement is superseded by the hierarchical
registration capability.
@jcfr jcfr force-pushed the remove-VRG-generation branch from 3ade5fc to 9f5e88c Compare November 18, 2024 16:28
@jcfr jcfr merged commit 5cd70dd into BrownBiomechanics:main Nov 18, 2024
2 checks passed
@sbelsk sbelsk deleted the remove-VRG-generation 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.

Remove obsolete collapsible group box related "VRG generation"
3 participants