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

Is this the behaviour we want? is_valid function returns input variable #50

Closed
eurunuela opened this issue Nov 28, 2019 · 1 comment
Closed
Labels
Discussion Discussion of a concept or implementation. Need to stay always open. Enhancement New feature or request

Comments

@eurunuela
Copy link
Collaborator

As I'm working on the unit tests, I've just seen that the is_valid function returns the input variable. Doesn't it make more sense for it to be a void function? If the variable is not valid, it could raise an error, but do nothing otherwise.

https://github.com/smoia/phys2bids/blob/f2d7d52e4e9d9d963f98aeb8809c81660e2f1794/phys2bids/physio_obj.py#L37

@eurunuela eurunuela added Enhancement New feature or request Discussion Discussion of a concept or implementation. Need to stay always open. labels Nov 28, 2019
@eurunuela eurunuela changed the title Is this the behaviour we want? is_valid function returns input value Is this the behaviour we want? is_valid function returns input variable Nov 28, 2019
@smoia
Copy link
Member

smoia commented Nov 28, 2019

Now it's like that because it's used during the assignment of properties in the init of our objects.
At the moment, there's fewer cases for it to assign to void than for assign to a variable.
This said, it's true that before either your or @rmarkello 's review in #38 it returned something depending on an input parameter (return_val=True).
For me it's a way to put it in a pipe, and it can be extended if we need it later on in a similar way to keep the code neat.
I also understand the idea behind this issue, so although I prefer this way it's not a strong preference.

@smoia smoia closed this as completed Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Discussion of a concept or implementation. Need to stay always open. Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants