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

Change prelude mag parameter to be an optional argument #178

Merged
merged 3 commits into from
Nov 25, 2020

Conversation

po09i
Copy link
Member

@po09i po09i commented Nov 20, 2020

Description

Fsl_prelude called by our prelude wrapper requires a mag input argument. If mag is not available or we don't want to mask the fieldmap, mag can be a matrix full of ones.

This PR changes the default behaviour of prelude to not require the mag argument and sets the default value of mag to be a matrix full of ones.

The tests and different calls to prelude were changed to call prelude appropriately.

@po09i po09i added the refactoring Improves code structure without affecting the user's functionality label Nov 20, 2020
@po09i po09i self-assigned this Nov 20, 2020
@po09i
Copy link
Member Author

po09i commented Nov 20, 2020

Relevant for #169

Copy link
Member

@jcohenadad jcohenadad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would just change the title of the PR to state that mag is now "optional" (vs. "default")

@po09i po09i changed the title Change prelude mag parameter to be a default argument Change prelude mag parameter to be a optional argument Nov 25, 2020
@po09i po09i changed the title Change prelude mag parameter to be a optional argument Change prelude mag parameter to be an optional argument Nov 25, 2020
@po09i po09i merged commit 552fbe4 into master Nov 25, 2020
@po09i po09i deleted the ad/prelude-mag-default branch November 25, 2020 17:23
kousu pushed a commit that referenced this pull request Mar 20, 2021
* Move mag to non default parameter

* Modify calls to prelude, make tests faster

* Move mag comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Improves code structure without affecting the user's functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants