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.

Sign up for GitHub

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

Jump to bottom

3.1.0 update #2812

Closed
wants to merge 582 commits into from
Closed

3.1.0 update #2812

wants to merge 582 commits into from

Conversation

Lestropie
Copy link
Member

This draft PR will show the set of changes to be applied as the 3.1.0 update, and track progress of manual integration of updates into that changeset.

The plan is that 3.1.0 will contain the set of changes currently on dev, as well as a number of active PRs not yet merged to dev; however it will not include the transition to cmake in #2689. This may mean that it will also omit the clang-format changes in #2652, though I'll need to bear this out.

Here's what I think the process will look like:

  1. Initial tip of the dev_sans_cmake branch is 8672d5b; this is the merge of PR remove TIFF support from core library #2720. I determined this by looking at the parents of merge commit 05878e5, which was the acceptance of Change build system to CMake #2689.

  2. Identify set of PRs that have been merged onto dev following the merge of Change build system to CMake #2689.

  3. For each of the items in the list determined from point 2 above:

    1. Do a merge onto the dev_sans_cmake branch.

  4. For currently open PRs targeting dev:

    1. Resolve against dev.
    2. Merge to dev.
    3. Manually merge to dev_sans_cmake.

bjeurissen and others added 30 commits October 25, 2022 15:25
@bjeurissen
Update docs
29eaf43
@bjeurissen
Fix duplicate author and synopsis and fix description
de53159
@bjeurissen
Update dwi2tensor.rst
98abc83
@jdtournier
Merge remote-tracking branch 'github/master' into dev
0b41219
@Lestropie
Merge pull request #2475 from MRtrix3/python_stack_tweak
5731c02 
python library: tweak execute to work with VS Code #2472
@jdtournier
pylint: remove unused or outdated options
8d53707 
Specifically, remove the options mentioned in the log:

testing/pylint.rc:1:0: E0015: Unrecognized option found: argument-name-hint, attr-name-hint, class-attribute-name-hint, class-name-hint, const-name-hint, function-name-hint, inlinevar-name-hint, method-name-hint, module-name-hint, variable-name-hint, no-space-check (unrecognized-option)
testing/pylint.rc:1:0: R0022: Useless option value for '--disable', 'bad-whitespace' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
@jdtournier
pylint: fix grep regex
7cebcc0 
This is to address this warning:

    grep: warning: stray \ before /
@jdtournier
pylint: fix various warnings
7611b59
@jdtournier
Merge pull request #2524 from MRtrix3/dev_update
53383aa 
Dev update
@jdtournier
Merge pull request #2471 from MRtrix3/population_template_space_init_…
84e11a4 
…drift_fix

population_template initialisation and drift correction fixes
@jdtournier
Merge branch 'dev' into cwlls_and_dkimetrics
74b3020
@dchristiaens
dwi2tensor: ignore NaNs also for IWLS (bug fix)
31c5a67
@dchristiaens
Merge branch 'dev' into dwi2tensor_ignore_nan
e191365
@dchristiaens
maskfilter: fix default value of minsize
efc716e
@dchristiaens
Merge branch 'dev' into connected_components_minsize
b51eaa8
@bjeurissen
Add options to override directions used for numerical evaluation of c…
be4826c 
…onstraints and metrics
@bjeurissen
Fix man page
45b66f4
@bjeurissen
Update dwi2tensor.rst
f5549a0
@bjeurissen
Update tensor2metric.rst
d848251
@bjeurissen
Update tensor2metric.cpp
696f5b6
@bjeurissen
Update tensor2metric.rst
bc67c09
@bjeurissen
Clean up man page
6f464de
@bjeurissen
Update docs
66def77
@jdtournier
add tests for DKI
79bca06
@jdtournier
dwi2tensor/tensor2metric: minor cosmetic changes
524df65
@jdtournier
tensor2metric: use OptionGroups to organise options
2083b41
@jdtournier
tensor2metric: performance improvements (avoid heap allocations)
8051824
@jdtournier
dwi2tensor: use abs-based testing for constrained fit
5101773
@jdtournier
configure: fix Qt parameter parsing for MacOS
ffe5891
@jdtournier
configure: ditch -flto flag from Qt qmake parameters
52fce59
@Lestropie Lestropie added this to the 3.1.0 updates milestone Feb 20, 2024
@Lestropie
Copy link
Member Author

It looks as though the base commit I chose does include the clang-format changes. #2652 was merged via 5b0ec65 2023-08-24, while #2689 was merged via 05878e5 2023-10-19. This is the opposite order to what I had expected. I think this probably gives weight to inclusion of clang-format changes in 3.1.0, given this may also make the various manual merges from dev to dev_sans_cmake easier.

@Lestropie
Copy link
Member Author

Lestropie commented Feb 20, 2024 •
edited
Loading

List below is relevant PRs merged onto dev since #2689 at time of posting.
Found by creating proposal PR from dev to dev_sans_cmake, looking for merge commits, omitting anything cmake-related.
Can tick off checkboxes as they are manually merged into dev_sans_cmake.

Unsure whether clang-tidy can be utilised in the absence of cmake:

daljit46 and others added 14 commits February 20, 2024 11:53
@daljit46 @Lestropie
Replace MR::unpack with std::apply
22e8200
@daljit46 @Lestropie
Reimplement MR::apply using std::apply
50be1a8
@daljit46 @Lestropie
Rename MR::apply to MR::apply_for_each
c8dcc5d
@MRtrixBot @Lestropie
Update copyright to 2024 on dev
a88e5e2
@MRtrixBot @Lestropie
Replace MR::vector and MR::deque with std types
39240ad 
Conflicts:
	cmd/mrview.cpp
	src/gui/mrview/tool/tractography/tractogram.h
@MRtrixBot @Lestropie
Replace MR::make_unique/shared with std::make_unique/shared
0afe37b
@daljit46 @Lestropie
Remove syntax checking for make_shared and vector
ada1e17
@daljit46 @Lestropie
Replace vector with array in axes_on_write
eb3000b 
Since we know the size of the mask needs to be 3 for NiFTI, array makes more sense.
@Lestropie
Fix memory leaks related to PNG handling
df409ee
@Lestropie
PNG: Use smart pointer on write
0240f0d
@daljit46 @Lestropie
Fix memory leak in FFT1D
cae27a9
@jdtournier @Lestropie
Update fft.h following suggestions from clang-tidy
1f0a3ae
@daljit46 @Lestropie
Fix memory leak in sh2peaks
b1d05c7 
The precomputer instance in Processor was allocated as a raw pointer and thus never freed.
@Lestropie
mrmath: Fix unfreed memory
4f3836b 
Resolves #2781.

Conflicts:
	core/misc/voxel2vector.h
@Lestropie
Copy link
Member Author

I have addressed the list of PRs merged to dev subsequently to #2689 using cherry-picking. It's possible that instead doing this using rebasing may be preferable from the perspective of cleanliness of commit history for 3.2.0, when dev will be getting merged to master and that will include the contents of dev_sans_cmake after this PR is merged. However I'm inexperienced with rebasing. I'll give it a go and see if I can make it work, and if so this pranch can be force-pushed.

@Lestropie
Fix mrview compilation through conflict resolution
72e03bb 
- Remove code within cmd/mrview.cpp that originates from 6417a52 as part of #2689, which was erroneously added in 39240ad.
- Fix compilation of src/gui/mrview/file_open.cpp to coincide with changes in #2764; these were omitted from cherry-picks 39240ad and 0afe37b due to that file having already been removed during the course of #2689.
@Lestropie
mrmath: Fix compilation
fed589b 
Resolves changes made to cmd/mrmath.cpp in #2784 to prevent memory leak against removal of MR::vector in #2689.
@Lestropie
run_tests: Write to testing/log/*.log
a41514d
@Lestropie
core/misc/voxel2vector.h: Fix compilation
4a51096 
Resolves changes made to fix unfreed memory in mrmath in #2784 against removal of MR::vector in #2764.
@Lestropie
configure: Enforce C++17
c6ab324 
Construction of a 3.1.0 release involves taking features on the development branch and separating them from the adoption of cmake in #2689. Some of these changes involve the use of C++17 features. However the transition to defaulting to C++17 was performed exclusively within the cmake ecosystem. This commit modifies the legacy "configure" script to reflect the increment to use of C++17.
@tclose
Copy link
Contributor

tclose commented Feb 25, 2024

What is the reasoning behind dropping cmake from this release?

@Lestropie
Copy link
Member Author

Reasoning included regular ongoing issues on dev, requirement to change Python executable script handling (#2741), desire to alter packaging for distribution (#2807), possibility of further reorganisation of filesystem structure, lack of solution for external "projects" / "extensions" (ie. C++ code that compiles against the shared library / Python scripts that execute against the API). However it looks like we are now aborting this prospect and are going to include cmake in 3.1.0.

@Lestropie Lestropie closed this Feb 26, 2024
@Lestropie Lestropie deleted the dev_sans_cmake branch June 22, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.1.0 updates
Development

Successfully merging this pull request may close these issues.
10 participants
@Lestropie @tclose @bjeurissen @jdtournier @dchristiaens @maxpietsch @MRtrixBot @Ruobing-Chen @Arshiyasan @daljit46