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

Support wrapping uninstantiated datamodel nodes #294

Merged

Conversation

johningve
Copy link
Contributor

Previously we just hardcoded the Settings/Acquisition and Settings2D/Acquisition nodes. This commit makes it work for any uninstantiated node that is part of a datamodelList by recursively searching the datamodel.

@johningve johningve requested a review from a team as a code owner November 27, 2024 14:00
@johningve johningve force-pushed the MISC-2024-11-27-properly-support-uninstantiated-datamodels branch 2 times, most recently from 7ba28c0 to aedd11b Compare November 28, 2024 08:08
vawale
vawale previously approved these changes Nov 28, 2024
Copy link
Contributor

@vawale vawale left a comment

Choose a reason for hiding this comment

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

minor comments

src/include/ZividPython/DataModelWrapper.h Outdated Show resolved Hide resolved
Comment on lines +344 to +347
if constexpr(isDirectChild(DM::path, ValueTypeContained::path))
{
wrapDataModel<false>(dest, ValueTypeContained{}, true);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is okay for now. I wonder if it makes better sense to have ValueTypeContained defined as part of DM::Descendants for leafDataModelList types. For example, defining using Settings::Acquisitions::Descendants = std::tuple<Settings::Acquisition>

Previously we just hardcoded the Settings/Acquisition and
Settings2D/Acquisition nodes. This commit makes it work for any
uninstantiated node that is part of a datamodelList by recursively
searching the datamodel.
@johningve johningve force-pushed the MISC-2024-11-27-properly-support-uninstantiated-datamodels branch from aedd11b to 59373cb Compare November 28, 2024 11:01
@johningve johningve merged commit 7c396d2 into master Nov 28, 2024
21 checks passed
@johningve johningve deleted the MISC-2024-11-27-properly-support-uninstantiated-datamodels branch November 28, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants