search

daducci / COMMIT

Linear framework to combine tractography and tissue micro-structure estimation with diffusion MRI
Other
43 stars 33 forks source link

Verbose parameter should be bool #90

Closed daducci closed 3 years ago

daducci commented 3 years ago

In the fit() function (and all functions inside solvers.py called from there) the parameter verbose is an integer whose value is 0 or 1, and its value is checked as if verbose>=1. I think it would be more intuitive to set verbose to be a boolean.

MarioOcampo commented 3 years ago

Yes, for the actual functionality, it could be more intuituve. If it is integer, it could have different levels of display, like None, important_steps (start, end), every_iteration, and others. But right now I think this is not implemented. This type of behavior can be implemented with modules like logging.

daducci commented 3 years ago

No, indeed, at the moment it is actually binary (yes or no) but implemented as an integer, which could introduce ambiguities. What if we change it to bool for the time being?

MarioOcampo commented 3 years ago

I think the change is possible, and this should not break actual pipelines.
Later we can think to have levels of verbose (logging)

daducci commented 3 years ago

I agree, let's keep this in mind. Can you create a PR to implement this small modification? No need to update the version, just the changelog

daducci commented 3 years ago

Clsoe by #96