SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
58 stars 29 forks source link

sirf.Reg input parsing error handling #883

Open AnderBiguri opened 3 years ago

AnderBiguri commented 3 years ago

Most sirf.Reg handles input parsing by raising AssertionError().

I see 2 things that may need improvement, but please do let me know if I am wrong:

  1. AssertionError() is an error from when an assert (i.e. something that should always be true, and enforced by the code) fails. Input parsing does not fit into this description, so I think the correct error to raise is TypeError(). This of course, is not a super important thing.
  2. There is no feedback to the user. I think the error should inform the user what it is supposed to input, and what it did input. e.g. https://github.com/SyneRBI/SIRF/blob/c276144b22950a755bcfef7f693fc1a94e582d8e/src/Registration/pReg/Reg.py#L1097-L1098 should read:
    if not isinstance(floating_image, SIRF.ImageData): 
     raise some_error_handler("Floating image should be of type SIRF.ImageData but its pf type "+ type(floating_image) +" instead.")

    or something like that.

KrisThielemans commented 3 years ago

agreed!