MagneticResonanceImaging / MRIReco.jl

Julia Package for MRI Reconstruction
https://magneticresonanceimaging.github.io/MRIReco.jl/latest/
Other
85 stars 22 forks source link

correct dimension input for non-square matrix #144

Closed aTrotier closed 1 year ago

aTrotier commented 1 year ago

Related to #143

codecov-commenter commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (2856ec3) 65.67% compared to head (283a719) 65.67%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #144 +/- ## ======================================= Coverage 65.67% 65.67% ======================================= Files 9 9 Lines 268 268 ======================================= Hits 176 176 Misses 92 92 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tknopp commented 1 year ago

Looks good.

One question though: Why did you add dependencies to MRIReco in Project.toml? These should not be necessary.

aTrotier commented 1 year ago

It was added when building the documentation offline.

I still have some errors during CI for the documentation. I think the subpackages are not dev ?

aTrotier commented 1 year ago

Now it seems to work except for that :

┌ Warning: failed to run `@example` block in src/generated/examples/01-binning.md:122-126
│ ```@example 01-binning
│ io = IOBuffer();
│ versioninfo(io);
│ split(String(take!(io)), '\n')
│ ```
│   value = UndefVarError: versioninfo not defined
└ @ Documenter.Expanders ~/.julia/packages/Documenter/pjwqp/src/Expanders.jl:563
[ Info: CrossReferences: building cross-references.
[ Info: CheckDocument: running document checks.

Maybe we can merge and see if it works when the documentation are deployed ?

aTrotier commented 1 year ago

@JeffFessler Thanks

aTrotier commented 1 year ago

@tknopp I think it is ready to merge :)

tknopp commented 1 year ago

I still don't understand why dependencies have been added to Project.toml: https://github.com/MagneticResonanceImaging/MRIReco.jl/pull/144/files#diff-72ed386c2a0cd1d23c0968297e70023ed98c22490d146dd89fc91f48369bad4d The dependencies are not used within MRIReco.

JeffFessler commented 1 year ago

I bet those dependencies belong in docs/Project.toml...