Skip to content

ENH: Update to NIFTILIB3.0.1#2630

Merged
thewtex merged 3 commits into
InsightSoftwareConsortium:masterfrom
gdevenyi:nifticlib3.0
Jul 9, 2021
Merged

ENH: Update to NIFTILIB3.0.1#2630
thewtex merged 3 commits into
InsightSoftwareConsortium:masterfrom
gdevenyi:nifticlib3.0

Conversation

@gdevenyi
Copy link
Copy Markdown
Contributor

@gdevenyi gdevenyi commented Jul 2, 2021 •
edited
Loading

Update embedded ITK version of NIFTILIB from snapshot of master to latest 3.0.1.

Fixes #1924

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions Bot added area:ThirdParty Issues affecting the ThirdParty module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Jul 2, 2021
@gdevenyi gdevenyi changed the title ENH: Update to NIFTILIB3.0.01 ENH: Update to NIFTILIB3.0.1 Jul 2, 2021
@gdevenyi gdevenyi marked this pull request as ready for review July 2, 2021 21:00
@gdevenyi
Copy link
Copy Markdown
Contributor Author

gdevenyi commented Jul 3, 2021

CC @hjmjohnson this swap in passes all tests. zlib-ng testing is stalled due to internal ITK zlib actually being heavily modified source.

dzenanz
dzenanz reviewed Jul 5, 2021
View reviewed changes
Copy link
Copy Markdown
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.

There are some style complaints from ghostflow:
https://github.com/InsightSoftwareConsortium/ITK/pull/2630/checks?check_run_id=2975705588
but this looks great!

Comment thread Modules/ThirdParty/NIFTI/UpdateFromUpstream.sh
@gdevenyi
Copy link
Copy Markdown
Contributor Author

gdevenyi commented Jul 5, 2021

There are some style complaints from ghostflow:
#2630 (checks)
but this looks great!

How does this work here, these are defects upstream, are we expected to fix them?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Jul 5, 2021

trailing whitespace and blank line at EOF fixes will probably be accepted upstream? But as the message itself says, the warnings don't HAVE to be fixed. And as zlib->zlib-ng swap is already getting quite a big effort, we could leave these fixes for the future?

@gdevenyi
Copy link
Copy Markdown
Contributor Author

gdevenyi commented Jul 5, 2021

trailing whitespace and blank line at EOF fixes will probably be accepted upstream? But as the message itself says, the warnings don't HAVE to be fixed. And as zlib->zlib-ng swap is already getting quite a big effort, we could leave these fixes for the future?

@hjmjohnson is an upstream developer and so may be interested in the style reports of https://github.com/InsightSoftwareConsortium/ITK/pull/2630/checks?check_run_id=2975705588 then

@thewtex thewtex requested a review from hjmjohnson July 7, 2021 20:30
thewtex
thewtex reviewed Jul 7, 2021
View reviewed changes
Copy link
Copy Markdown
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.

@gdevenyi thanks!!

It looks like there are support script that we do not want / need from the the upstream repository. Could they be removed in UpdateFromUpstream.sh?

Comment thread Modules/ThirdParty/NIFTI/src/nifti/nifti2/nifti_regress_test/commands/c21.d.misc.tests Outdated
gdevenyi and others added 3 commits July 8, 2021 09:12
@gdevenyi
ENH: Prepare for update to NIFTILIB3.0.1
68221fd
@gdevenyi
nifti 2020-08-07 (96b9954f)
9c2c9fa 
Code extracted from:

    https://github.com/NIFTI-Imaging/nifti_clib.git

at commit 96b9954fafcf3bd69a4ee2091b0d707ed2077697 (v3.0.1).
@gdevenyi
Merge branch 'upstream-nifti' into nifticlib3.0
faa754c 
# By NIFTI Upstream
* upstream-nifti:
  nifti 2020-08-07 (96b9954f)
@gdevenyi
Copy link
Copy Markdown
Contributor Author

gdevenyi commented Jul 8, 2021

Good catch @thewtex, I have no idea why nifti2 directrory was originally included, we disable nifti2 anyways.

@thewtex
Copy link
Copy Markdown
Member

thewtex commented Jul 9, 2021

@gdevenyi thanks!! ⭐

@thewtex thewtex merged commit cf3cea5 into InsightSoftwareConsortium:master Jul 9, 2021
@gdevenyi gdevenyi deleted the nifticlib3.0 branch December 24, 2021 15:07
@jcfr jcfr mentioned this pull request Feb 24, 2022
Closed
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 12, 2024
@N-Dekker
STYLE: No longer set hooks-max-size for nifti2_io.c, doxygen.config.in
7be11a1 
nifti2_io.c was removed by:
- pull request InsightSoftwareConsortium#2630
commit 68221fd
"ENH: Prepare for update to NIFTILIB3.0.1", 9 July 2021

The two doxygen.config.in files were removed by:
- pull request InsightSoftwareConsortium#3311
commit a96d03f
"COMP: Use modern doxygen_add_docs command ITK_WRAP_DOCS", 11 March 2022
- pull request InsightSoftwareConsortium#4500
commit 0c5c638
"STYLE: Remove doxygen.config.in (superseded by DoxygenConfig.cmake)", 12 March
2024
@N-Dekker N-Dekker mentioned this pull request Mar 12, 2024
Merged
hjmjohnson pushed a commit that referenced this pull request Mar 23, 2024
@N-Dekker @hjmjohnson
STYLE: No longer set hooks-max-size for nifti2_io.c, doxygen.config.in
88bfbfd 
nifti2_io.c was removed by:
- pull request #2630
commit 68221fd
"ENH: Prepare for update to NIFTILIB3.0.1", 9 July 2021

The two doxygen.config.in files were removed by:
- pull request #3311
commit a96d03f
"COMP: Use modern doxygen_add_docs command ITK_WRAP_DOCS", 11 March 2022
- pull request #4500
commit 0c5c638
"STYLE: Remove doxygen.config.in (superseded by DoxygenConfig.cmake)", 12 March
2024
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@thewtex
Merge pull request InsightSoftwareConsortium#2630 from gdevenyi/nifti…
852784b 
…clib3.0
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@N-Dekker @hjmjohnson
STYLE: No longer set hooks-max-size for nifti2_io.c, doxygen.config.in
ef5e496 
nifti2_io.c was removed by:
- pull request InsightSoftwareConsortium#2630
commit 5e64b4b
"ENH: Prepare for update to NIFTILIB3.0.1", 9 July 2021

The two doxygen.config.in files were removed by:
- pull request InsightSoftwareConsortium#3311
commit edae38c
"COMP: Use modern doxygen_add_docs command ITK_WRAP_DOCS", 11 March 2022
- pull request InsightSoftwareConsortium#4500
commit 8098f8b
"STYLE: Remove doxygen.config.in (superseded by DoxygenConfig.cmake)", 12 March
2024
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
@thewtex
Merge pull request InsightSoftwareConsortium#2630 from gdevenyi/nifti…
91b7a14 
…clib3.0
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
@N-Dekker @hjmjohnson
STYLE: No longer set hooks-max-size for nifti2_io.c, doxygen.config.in
4799b24 
nifti2_io.c was removed by:
- pull request InsightSoftwareConsortium#2630
commit 5895c80
"ENH: Prepare for update to NIFTILIB3.0.1", 9 July 2021

The two doxygen.config.in files were removed by:
- pull request InsightSoftwareConsortium#3311
commit 261b17a
"COMP: Use modern doxygen_add_docs command ITK_WRAP_DOCS", 11 March 2022
- pull request InsightSoftwareConsortium#4500
commit 8859699
"STYLE: Remove doxygen.config.in (superseded by DoxygenConfig.cmake)", 12 March
2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thewtex thewtex thewtex approved these changes

@dzenanz dzenanz dzenanz approved these changes

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

Assignees

No one assigned

Labels

area:ThirdParty Issues affecting the ThirdParty module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Integrate new nifticlib 3.0.0

3 participants

@gdevenyi @dzenanz @thewtex