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

Handling "irregular" MR acquisitions #1174

Merged
merged 10 commits into from
Feb 9, 2023
Merged

Conversation

evgueni-ovtchinnikov
Copy link
Contributor

@evgueni-ovtchinnikov evgueni-ovtchinnikov commented Feb 2, 2023

Changes in this pull request

In SIRF, some MR acquisitions are "irregular" and need to be excluded from certain operations on acquisition data objects, e.g. algebraic operations. A prime example is noise calibration data. This fact was not taken into account in some MRAcquisitionData methods dealing with "regular" data dimensions, leading to errors. As a quick fix, all "irregular" acquisitions are currently thrown away altogether when reading acquisitions data from a file. This PR aims to implement a more flexible approach whereby "irregular" acquisitions are only ignored where they have to be ignored.

Testing performed

Related issues

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added docstrings/doxygen in line with the guidance in the developer guide
  • I have implemented unit tests that cover any new or modified functionality
  • The code builds and runs on my machine
  • CHANGES.md has been updated with any functionality change

Contribution Notes

Please read and adhere to the contribution guidelines.

Please tick the following:

  • The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in SIRF (the Work) under the terms and conditions of the Apache-2.0 License.

@evgueni-ovtchinnikov evgueni-ovtchinnikov marked this pull request as ready for review February 3, 2023 16:45
@evgueni-ovtchinnikov
Copy link
Contributor Author

@KrisThielemans Codacy complains about an issue that does not make any sense - can I just ignore it and merge this PR?

@KrisThielemans
Copy link
Member

Codacy complains about an issue that does not make any sense - can I just ignore it and merge this PR?

sure. that message makes no sense at all.

I haven't reviewed the code. I understand @johannesmayer is happy, so I'm happy!

Maybe just update the copyright dates in the header of each file before merging.

thanks!

@evgueni-ovtchinnikov
Copy link
Contributor Author

copyright notices updated, merging...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants