-
Notifications
You must be signed in to change notification settings - Fork 26
Downsample register example #342
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
base: main
Are you sure you want to change the base?
Conversation
For deployment, we also needed to add pip to the build environment.
Latest version.
Note: we support std::variant for the ValueVector to support, for example, the actual boolean or number types being specified in python or typescript.
1fb071f
to
fc0c7cb
Compare
Simple and fast.
More efficient and enables use in bindings.
More efficient, enables bindings, and improves wasm module size.
aee7c1e
to
f5f8aff
Compare
import { downwsampleNode } from "@itk-wasm/downsample"; | ||
import { elastixNode } from "@itk-wasm/elastix"; | ||
|
||
const fixedImageFile = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my understanding (as I don't know MJS) what does it mean to do const fixedImageFile =
without an expression at the right-hand-side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is incomplete :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My very first micro-contribution to this PR 😸
We still have the issue:
Should we be adding the output spacing as a parameter? |
Just checked: I think the idea is that it important that users explicitly specify the spacing, rather than leave it (1, 1, 1), for example, because for realistic images, it is very relevant. Right, Stefan and Marius (@stefanklein, @mstaring)? |
* @param parameterObject Pointer to the elastix::ParameterObject to populate | ||
* @return std::string Error message if reading the parameter object fails, empty string otherwise. | ||
*/ | ||
std::string readParameterObject(const std::string & parameterObjectJson, elastix::ParameterObject * parameterObject) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Micro-nitpick: elastix basically follows the ITK naming convention, so function names should preferably start with a capital letter. See https://github.com/InsightSoftwareConsortium/ITKSoftwareGuide/blob/96450bf1b8a882b02d416a5929e44301b59acfe3/SoftwareGuide/Latex/Appendices/CodingStyleGuide.tex#L733
So I think it should be ReadParameterObject
, rather than readParameterObject
. (Otherwise please propose changing the ITK naming convention 😸!)
f5f8aff
to
8410990
Compare
const auto & valueDouble = std::get<double>(value); | ||
parameterValues.push_back(std::to_string(valueDouble)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::to_string
is not lossless for floating point, please consider using ITK's NumberToString instead:
parameterValues.push_back( ConvertNumberToString(std::get<double>(value)) )
Resamples the input with the transform.
8410990
to
6cb5d47
Compare
No description provided.