Checking whether given fields match PW internal field names sometimes has side-effect
mzy2240 opened this issue · 3 comments
Currently, it is internally mandatory to check whether the given fields match PW internal fields, however, this may prevent users access some existing but somehow not included internal fields. For example, 'LODFMult:x' (x can be any integer from 0 to the index of the last branch) is a valid field for branch object to query result from LODF matrix. You could access it using original SimAuto with/without running CalculateLODFMatrix in advance. But you cannot query it using ESA since it is not included in the referenced field list. I am pretty sure there will be other fields have the same problem as LODFMult. A solution could be adding an optional argument to disable field matching when calling GetParameter series functions. Do you have any better solution in your mind @blthayer ?
I looked a bit at the code, and it might be a bit tricky. Part of ESA/SAW's job is to handle type conversions, and the error you provided crops up when identifying whether or not a field is intended to be numeric. So, one fix (that might have unintended consequences) would be to overhaul how numeric fields are identified. You could essentially assume all fields are numeric and use some try/except block combined with pandas.to_numeric instead of the more complex method that's used right now (look up the PowerWorld field and its type, attempt to cast accordingly).
Alternatively, instead of strictly matching (or, after attempting to strictly match) the PowerWorld field like what's done now, you could use regular expressions (built-in re module) to check that it's a "PowerWorld-like" field. The regular expression might look something like rf"${}(:[0-9]+)?".format("LODFMult") for your example, where the "LODFMult" part would be dynamic. Please note I have not tested this expression, and don't know all the variations that the PowerWorld fields could have 😄
As I stated on the last issue, the trick is maintaining backwards compatibility now that (presumably) a good handful of other folks are using ESA. Be careful that you're confident that updates won't change the behavior of user code before shipping anything 😄