tBuLi / symfit

Symbolic Fitting; fitting as it should be.
http://symfit.readthedocs.org
MIT License
233 stars 17 forks source link

Fixes issue 295 #296

Open Jhsmit opened 4 years ago

Jhsmit commented 4 years ago

Checks if any bounds are none before comparing min/min

For some reason there are some commits listed which are already on symfit master

Jhsmit commented 4 years ago

Edited on GitHub which also added a newline at the end of the file

pckroon commented 4 years ago

Ok, but then you also need to add the Parameter.min and Parameter.max properties... Currently we'd just lose a useful check instead of moving it.

Jhsmit commented 4 years ago

The current check I've removed was redundant because its also in the __init__ of Parameter, where also the possibility of None is checked in one line.

In any case, the getter/setter would still add functionality because it prevents users from setting faulty limits after initialization of the parameters.

pckroon commented 4 years ago

parameters does not call Parameter.__init__. Instead it does for param, value in zip(params, values): (param, key, value) (https://github.com/tBuLi/symfit/pull/296/files#diff-f73c0cdf32475d7606d6d487cae94340R245).

Of course, once there's setter methods the check in Parameter.__init__ will become redundant.

Jhsmit commented 4 years ago

Ah I see, Parameter.__init__ does get called but not with the user supplied min/max values which indeed are set later. I'll put in the setter methods instead.

tBuLi commented 4 years ago

Yeah I agree that putting this check in the setter is probably a nicer solution. Another solution could be to initiate each object properly and not set it after the fact.