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.

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

Fixing issues in Julia 1.11 and Julia 1.12 #422

Merged
merged 7 commits into from
Jul 5, 2024
Merged

Conversation

cncastillo
Copy link
Member

@cncastillo cncastillo commented Jul 3, 2024

The incorrect definition of the getproperty function for Array{Grad/RF/ADC} generated some weird errors. This PR fixes these issues.


I also included [workspace]'s to the Project.toml's. A feature that will be introduced in Julia 1.12 JuliaLang/Pkg.jl#3841. The cool thing is that our mono repo and test setups are greatly simplified. For the KomaMRICore tests, the test_args are just args:

❯ julia --project=test --threads=4 test/runtests.jl
[ Info: Testing on the CPU with 4 thread(s)
...

and for CUDA (with CUDA added to the v1.12 environment)

❯ julia --project=test test/runtests.jl CUDA

This one does not work yet, due to JuliaGPU/GPUCompiler.jl#593.


The mono repo setup should be (I think?)

pkg> dev KomaMRI

Instead of dev'ing all the environments for each sub-package and their corresponding test folders.

Copy link

codecov bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 6 lines in your changes missing coverage. Please review.

Project coverage is 90.52%. Comparing base (3128e9f) to head (4c6ed16).
Report is 7 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
+ Coverage   90.45%   90.52%   +0.07%     
==========================================
  Files          51       51              
  Lines        2796     2787       -9     
==========================================
- Hits         2529     2523       -6     
+ Misses        267      264       -3     
Flag Coverage Δ
base 88.20% <84.21%> (+0.17%) ⬆️
core 90.37% <ø> (ø)
files 93.55% <100.00%> (ø)
komamri 93.98% <ø> (ø)
plots 89.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
KomaMRIBase/src/datatypes/Sequence.jl 92.47% <100.00%> (-0.14%) ⬇️
KomaMRIBase/src/sequences/PulseDesigner.jl 94.23% <100.00%> (+0.17%) ⬆️
KomaMRIFiles/src/Sequence/Pulseq.jl 92.89% <100.00%> (ø)
KomaMRIBase/src/datatypes/sequence/ADC.jl 94.59% <75.00%> (ø)
KomaMRIBase/src/datatypes/sequence/Grad.jl 87.87% <92.85%> (+1.11%) ⬆️
KomaMRIBase/src/datatypes/sequence/RF.jl 81.63% <55.55%> (+1.63%) ⬆️

... and 1 file with indirect coverage changes

@cncastillo
Copy link
Member Author

Apparently, the pre-release CI is failing due to problems in MRIFiles. Check: MagneticResonanceImaging/MRIReco.jl#194.

@cncastillo
Copy link
Member Author

I am going to merge this and look into MRIReco's problem later.

@cncastillo cncastillo merged commit eb05128 into master Jul 5, 2024
9 of 13 checks passed
@cncastillo cncastillo deleted the fix-type-piracy branch July 5, 2024 17:25
@cncastillo cncastillo mentioned this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant