Open FFroehlich opened 4 years ago
A possible fix would be to define types for all columns for all dataframes (should be {string, int, float}) with type conversion in the read-in https://github.com/PEtab-dev/PEtab/blob/master/petab/parameters.py#L15.
Turned out to be quite a pain with different column types, depending which specific values they have. Also with omitted optional columns, or omitted values and default. Shall we "normalize" everything on loading and expect that to have happened in all other places (we had the discussion some time before...)? Advantage: Reduced code complexity, probably less bugs; disadvantage: the read-back-files after writing may differ from the original ones if not already in "normalized" form...
Advantage: Reduced code complexity, probably less bugs; disadvantage: the read-back-files after writing may differ from the original ones if not already in "normalized" form...
In what could they differ? Int to float / string conversion?
In what could they differ? Int to float / string conversion?
parameterScale
absent, vs parameterScale
=lin
and stuff like that.
Well, some differences may occur already now, depending on which NA
value was used in the original files... But that I really wouldn't care about.
Ah, you mean about filling in default values. Yeah, not sure about that. Shouldn't hurt I guess to fill in all columns with their default values. We argued at some point about when reading in adding all those default values, which would make many functions easier (removing basically all i"if not exists then set to default value" lines). But no special opinion on this.
There is a function somewhere that does this, at least for a few data frames.
Well, some differences may occur already now, depending on which
NA
value was used in the original files... But that I really wouldn't care about.
No, NA vs "" vs whatever else pandas interpretes as empty should not matter.
When all
objectivePriorParameters
are either empty or set to a single value, the respective column will be read with dtypenumpy.float64
, which may cause problems down the line as spec says this should be a string. Not a issue currently since all currently available priors require two parameters, but may lead to problems if that changes.