Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

single objectivePriorParameters are cast to numpy.float64 #27

Open
FFroehlich opened this issue Aug 27, 2020 · 7 comments
Open

single objectivePriorParameters are cast to numpy.float64 #27

FFroehlich opened this issue Aug 27, 2020 · 7 comments

Comments

@FFroehlich
Copy link
Collaborator

When all objectivePriorParameters are either empty or set to a single value, the respective column will be read with dtype numpy.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.

@yannikschaelte
Copy link
Member

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.

@dweindl
Copy link
Member

dweindl commented Aug 27, 2020

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...

@yannikschaelte
Copy link
Member

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?

@dweindl
Copy link
Member

dweindl commented Aug 27, 2020

In what could they differ? Int to float / string conversion?

parameterScale absent, vs parameterScale=lin and stuff like that.

@dweindl
Copy link
Member

dweindl commented Aug 27, 2020

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.

@yannikschaelte
Copy link
Member

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.

@yannikschaelte
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants