GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
228 stars 107 forks source link

Fix PositionD parsing in config from strings like '123.1, 234.2', which had been doing the wrong thing. #1300

Closed rmjarvis closed 4 months ago

rmjarvis commented 4 months ago

@jmeyers314 noticed in #1299 that a string like '123.4, 567.8' doesn't get parsed correctly. This PR fixes the implementation to work correctly.

jmeyers314 commented 4 months ago

Although, it just occurred to me this will still fail if given:

pos : 12

=> PositionD(1.0, 2.0).

Maybe that's bizarre enough to not bother defending against though.

rmjarvis commented 4 months ago
pos : 12

That will give an appropriate error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/Mike/GalSim/galsim/config/value.py", line 206, in ParseValue
    val = _GetPositionValue(param)
  File "/Users/Mike/GalSim/galsim/config/value.py", line 463, in _GetPositionValue
    raise GalSimConfigError("Unable to parse %s as a PositionD. Caught %s"%(param,e))
galsim.errors.GalSimConfigError: Unable to parse 12 as a PositionD. Caught 'int' object has no attribute 'split'

The one that will work inappropriately is

pos : "12"

which I think is probably bizarre enough not to bother defending against.

jmeyers314 commented 4 months ago

Ah, right. Yaml sees that as an int, not a str, so all is well.

rmjarvis commented 4 months ago

Is there any urgency in getting this into a bugfix release? Or should we let this sit until we have more bugs fixed?

jmeyers314 commented 4 months ago

No urgency I think. The original workaround you proposed seems fine.