Skip to content

ENH: Add ITK_Example_TomlFileFormatForParameterFiles.ipynb #329

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

Conversation

N-Dekker
Copy link
Collaborator

@N-Dekker N-Dekker commented Mar 19, 2025

Demonstrates TOML support for elastix registration parameter files and transformation parameter files.


@thewtex @mstaring @stefanklein Please have a look!

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@N-Dekker N-Dekker force-pushed the Example_TomlFileFormatForParameterFiles-ipynb branch 2 times, most recently from 4e6f752 to 5ad9ea5 Compare March 21, 2025 13:49
@N-Dekker N-Dekker marked this pull request as ready for review March 21, 2025 13:52
@@ -0,0 +1,528 @@
{
Copy link
Member

@thewtex thewtex Mar 21, 2025

Choose a reason for hiding this comment

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

Line #8.        import pip._vendor.tomli as tomllib  # the same tomllib that's now included in Python v3.11+

Cool!


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thewtex Actually I also liked your suggestion to pip install tomllib for older versions of Python, but I couldn't remember how to do that exactly. 🤷

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

@N-Dekker well done ! 🎇

Demonstrates TOML support for elastix registration parameter files and
transformation parameter files.
@N-Dekker N-Dekker force-pushed the Example_TomlFileFormatForParameterFiles-ipynb branch from 5ad9ea5 to 5dfe125 Compare March 25, 2025 10:50
@N-Dekker
Copy link
Collaborator Author

FYI, this force-push sets the center of rotation of the initial transformation at (0.0, 0.0). I think that's nice, because now the initial transformation does not need to know about the image sizes. Instead, the images are now adjusted to have their centers at world coordinates (0.0, 0.0) as well.

@N-Dekker
Copy link
Collaborator Author

@thewtex Is it OK to you if we just merge this Jupyter Notebook PR now? The wasm build still fails, but that seems unrelated.

The wasm build has a few harmless warnings (warning: unused parameter 'filename' [-Wunused-parameter]). Do those warnings make the CI fail? Or is it the error message at the end? As follows:

Pixi task (bindgen-typescript): npx itk-wasm -b emscripten-build bindgen --interface typescript --output-dir $ITK_WASM_TYPESCRIPT_OUTPUT_DIR --package-name $ITK_WASM_TYPESCRIPT_PACKAGE_NAME --package-description "$ITK_WASM_DESCRIPTION" --repository "$ITK_WASM_REPOSITORY": (Generate typescript bindings)
Error:   × failed to parse glob expression

Anyway, it's clearly beyond the scope of this PR 😇

@thewtex
Copy link
Member

thewtex commented Mar 25, 2025

@N-Dekker yes, merge sounds good to me. I thought I fixed wasm CI in another PR. I will take another pass when we upgrade to v5.4.3.

@N-Dekker N-Dekker merged commit 51581d3 into InsightSoftwareConsortium:main Mar 25, 2025
19 of 20 checks passed
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.

2 participants