Closed jkupka closed 6 years ago
Added test, rewritten the function to try to cast the data type first
we need to keep in mind that int(10.5) is 10
so net shouldn't have int fields
We shouldn't be casting types because we risk losing informatioon in a case like in my previous comment. If the user changes sn_kva to int, it is the responsibility of the user. We can discuss this on Monday, now I will make the pull request so that the above example works and net can be loaded from json.
We shouldn't be casting types because we risk losing informatioon in a case like in my previous comment. If the user changes sn_kva to int, it is the responsibility of the user.
The task of the from_json / to_json functions should be to restore a grid exactly as it was saved, not to do data type checks or type casting. So if net.sn_kva is a string for some reason when to_json is applied, it should load again with net.sn_kva as a string.
I did not look into how the from_json / to_json functions were refactored, but if they expect certain data types in certain fields that is the wrong approach in my opinion. Maybe we need to roll back #139 and first discuss this more thoroughly?
new from_json/to_json looks good. it is definitely cleaner than before.
one suggestion:
Right now
to_json
doesn´t ensure right data types and sometimes it´s not possible to load the saved network withfrom_json
again.The following example will raise a data type mismatch error because the reference apparent power will be saved as int instead as float and
from_json
is expecting sn_kva to be a float.