Skip to content

ENH: Bump Python package version to 0.22.0 #326

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

Merged
merged 5 commits into from
Mar 8, 2025
Merged

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented Mar 6, 2025

And itk Python dep to 5.4.2.

@N-Dekker this is a step for publishing a new Python package version. And it allows us to test recent CI configuration improvements.

And itk Python dep to 5.4.2.
@thewtex thewtex requested a review from N-Dekker March 6, 2025 16:40
"itk~=5.4.0",
"itk~=5.4.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

SuperElastix/elastix still supports ITK 5.4.1, so I guess it might as well say "itk~=5.4.1", right?

Copy link
Member

Choose a reason for hiding this comment

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

There are no python packages generated for 5.4.1.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @dzenanz But if I understand correctly, the expression "itk~=5.4.1" would match both 5.4.1 and 5.4.2, even if 5.4.1 doesn't exist 😸

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we are building against itk 5.4.2, Bumping the compatible release specification is a good idea:

https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release

There are not necessarily binary compatibility issues, but this is a good practice.

thewtex added 3 commits March 6, 2025 13:30
In recent ITKRemoteModuleBuildTestPackageAction, these are built on a
separate native ARM CI system.
@thewtex thewtex requested a review from N-Dekker March 7, 2025 23:16
@N-Dekker
Copy link
Collaborator

N-Dekker commented Mar 7, 2025

Thanks Matt!

I'm sorry I don't know how to review these 1000+ code changes 🤷 Any suggestion?

In addition to building the WASM modules, build the Typescript bindings
and generate the JavaScript bundles.
@thewtex
Copy link
Member Author

thewtex commented Mar 8, 2025

@N-Dekker it is not necessary to validate all the changes in pixi.lock or pnpm-lock.yaml :-)

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

LGTM 😄

@N-Dekker
Copy link
Collaborator

N-Dekker commented Mar 8, 2025

it is not necessary to validate all the changes in pixi.lock or pnpm-lock.yaml :-)

Thanks, Matt. Are these two files then generated fully automatically?

@N-Dekker
Copy link
Collaborator

N-Dekker commented Mar 8, 2025

As far as I can see, all CI checks pass now, except for "Build / py-dev / test-linux-notebooks (pull_request)", at https://github.com/InsightSoftwareConsortium/ITKElastix/actions/runs/13738452046/job/38427626120?pr=326 saying:

RuntimeError: /work/build/cp39-cp39-linux_x86_64/_deps/elx-src/Core/Main/itkElastixRegistrationMethod.hxx:389:
ITK ERROR: ElastixRegistrationMethod(0x55650d993740): Internal elastix error: See elastix log (use LogToConsoleOn() or LogToFileOn()).
----------------------------- Captured stdout call -----------------------------
WARNING: The parameter "FixedInternalImagePixelType", requested at entry number 0, does not exist at all.
  The default value "float" is used instead.
WARNING: The parameter "MovingInternalImagePixelType", requested at entry number 0, does not exist at all.
  The default value "float" is used instead.
Installing all components.
InstallingComponents was successful.

ELASTIX version: 5.2.0
Git revision SHA: df075e8e1fd1a647cdda2c1e5a8a6febd9074ad0
Git revision date: Fri Feb 21 16:19:08 2025 -0500
Command line options from ElastixBase:
-threads  unspecified, so all available threads are used
WARNING: The parameter "UseDirectionCosines", requested at entry number 0, does not exist at all.
  The default value "true" is used instead.

WARNING: The option "UseDirectionCosines" was not found in your parameter file.
  From elastix 4.8 it defaults to true!
This may change the behavior of your registrations considerably.

Command line options from TransformBase:
-t0       unspecified, so no initial transform used
WARNING: The parameter "BSplineTransformSplineOrder", requested at entry number 0, does not exist at all.
  The default value "3" is used instead.

Reading images...
Reading images took 0 ms.

WARNING: the fixed pyramid schedule is not fully specified!
  A default pyramid schedule is used.
WARNING: the moving pyramid schedule is not fully specified!
  A default pyramid schedule is used.

itk::ExceptionObject (0x556505c35940)
Location: "unknown" 
File: /work/build/cp39-cp39-linux_x86_64/_deps/elx-src/Components/Metrics/VarianceOverLastDimension/elxVarianceOverLastDimensionMetric.hxx
Line: 83
Description: ITK ERROR: VarianceOverLastDimensionMetric(0x556505c33d80): 
ERROR: the direction cosines matrix of the fixed image is invalid!

  The VarianceOverLastDimensionMetric expects the last dimension to represent
  time and therefore requires a direction cosines matrix of the form:
       [ . . 0 ]
  dc = [ . . 0 ]
       [ 0 0 1 ]

But that's an issue you reported before: #291 So let's merge now! Thanks again Matt!

@N-Dekker N-Dekker merged commit 67ab36c into main Mar 8, 2025
23 of 24 checks passed
@thewtex
Copy link
Member Author

thewtex commented Mar 9, 2025

Are these two files then generated fully automatically?

👍 yes

Thanks @N-Dekker !

@N-Dekker
Copy link
Collaborator

N-Dekker commented Mar 9, 2025

@thewtex Can you please explain how to generate those files automatically? Maybe tomorrow... 😃

@thewtex thewtex deleted the python-package-version branch March 10, 2025 21:29
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.

3 participants