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

get_fsdb_info setting defaults that are not in the standard formula #233

Open
kuaka opened this issue Dec 27, 2020 · 3 comments
Open

get_fsdb_info setting defaults that are not in the standard formula #233

kuaka opened this issue Dec 27, 2020 · 3 comments
Labels

Comments

@kuaka
Copy link
Collaborator

kuaka commented Dec 27, 2020

get_fsdb_info is trying to read formula values from a fsdb file. When it does not find them it sometimes sets a default. It should be using what is already present in the formula object, which to my understanding was loaded from the formula file (e.g. gap2016.py etc). We should be keeping these defaults if the value is not present in the fsdb file. This was the cause of #227.

In the function I see that there are other "else" conditions and think that these should not be overiding what is already in the formula. If these are not present in the formula file then I think that they should be left as the formula object (should) have a default value anyway.

@kuaka kuaka added the fsdb label Dec 27, 2020
@biuti
Copy link
Collaborator

biuti commented Dec 27, 2020

This should be fixed with c9c82a0.

@kuaka
Copy link
Collaborator Author

kuaka commented Dec 28, 2020

I still see some else statements that force a default. Any defaults should be the ones in the formula file (gap2016.py etc)

formula.formula_time = 'on' if fsdb_data.get('use_time_points') == '1' else 'off'

formula.formula_time = 'on' if fsdb_data.get('use_time_points') == '1' else 'off'

for example this sets time points to off if use_time_points is missing or not 1. It may never happen that this is missing in fsdb but that would be making an assumption.
IMHO it would be better to code it something like this:

if fsdb_data.get('use_time_points') == '1' 
    formula.formula_time = 'on'

or

if fsdb_data.get('use_time_points') == '1':
   formula.formula_time = 'on'
elif  fsdb_data.get('use_time_points') == '0':
   formula.formula_time = 'off'

that way we are not overiding the default in the formula file in all other cases.

@biuti
Copy link
Collaborator

biuti commented Dec 28, 2020

I left the binaries / fixed values always present, like formula_position.
We can change but if FSDB format will change so much to be possibly missing or with different values, function would have to be rewritten anyway.

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

No branches or pull requests

2 participants