Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates major dependencies across the MRD project, including Python 3.9 → 3.13, ISMRMRD 1.14.2 → 1.15.0, HDF5 1.10 → 1.14, xtensor 0.25.0 → 0.27.1, and Yardl 0.6.4 → 0.6.6. It also migrates C++ compilation to C++20, removes xtensor-fftw in favor of direct FFTW3 usage, replaces the fmt library with std::format, and updates deprecated Python datetime APIs.
Changes:
- Bump Python from 3.9 to 3.13 and update deprecated datetime APIs
- Update C++ dependencies (ISMRMRD, HDF5, xtensor) and implement custom FFTW wrappers replacing xtensor-fftw
- Migrate to C++20 and replace fmt library with std::format
- Update build configurations to remove mamba references (now built into conda)
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| environment.yml | Update conda dependencies including Python, ISMRMRD, HDF5, xtensor, and remove xtensor-fftw |
| cpp/CMakeLists.txt | Update default C++ standard to C++20 |
| cpp/mrd/CMakeLists.txt | Add C++17 requirement for generated code |
| cpp/conda/meta.yaml | Update C++ package dependencies |
| cpp/conda/build.sh | Set C++ standard to 20 for conda builds |
| cpp/mrd-tools/fftw_wrappers.h | New file implementing FFTW wrapper functions to replace xtensor-fftw |
| cpp/mrd-tools/CMakeLists.txt | Remove fmt dependency, add warning suppression for GCC 15+ |
| cpp/mrd-tools/*.cc | Update includes for xtensor 0.27.1 and replace xtensor-fftw with custom wrappers |
| cpp/mrd/yardl/detail/ndarray/impl.h | Add version-conditional includes for xtensor compatibility |
| python/mrd/yardl_types.py | Replace deprecated utcfromtimestamp with fromtimestamp(tz=UTC) |
| python/mrd/_binary.py | Replace deprecated datetime API and use complex instead of ComplexFloat |
| python/mrd/tools/phantom.py | Refactor acquisition creation to use factory function |
| python/mrd/types.py | Update type annotations for Python 3.13 compatibility |
| python/mrd/_dtypes.py | Add Annotated type support |
| .github/workflows/mrd_build.yml | Update Yardl version to 0.6.6 |
| .github/actions/configure-mrd-build-environment/action.yml | Remove use-mamba flag |
| docker/Dockerfile | Update base image to Ubuntu 24.04, update Miniforge, replace mamba with conda |
| .devcontainer/Dockerfile | Update Miniforge and Yardl versions, replace mamba with conda |
| docs/*/quickstart.md | Update installation instructions to use conda instead of mamba |
| VERSION | Bump version from 2.1.2-preview to 2.2.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johnstairs
left a comment
There was a problem hiding this comment.
Should we also increase the required Python version as well in pyproject.toml?
|
I don't think so actually. The MRD Python library is still compatible with Python 3.9, so it advertises that it requires any Python >= 3.9. The changes here enable us to use Python 3.13 in the devcontainer and CI pipeline (which, for example, uncovered a bug in What do you think? |
johnstairs
left a comment
There was a problem hiding this comment.
Yeah, that's fine. At some point, we can bump it to take advantage of newer features, but it doesn't have to be now. Should we have testing that verifies that it works with older versions?
|
OK, yes, backward compatibility with Python should be tested. Instead, I'll do what you first suggested. MRD will require Python >= 3.13. I think this is appropriate. |
mamba(libmamba-solver is built into Conda now)