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

Simplify parameter boundaries interface for fisignal #40

Closed
luisfabib opened this issue Oct 19, 2020 · 2 comments · Fixed by #71
Closed

Simplify parameter boundaries interface for fisignal #40

luisfabib opened this issue Oct 19, 2020 · 2 comments · Fixed by #71
Assignees
Labels
Milestone

Comments

@luisfabib
Copy link
Collaborator

Another remnant of the old MATLAB interface. In fitsignal the lower/upper boundaries of the different parameter subsets are specified as a single keyword

fitsignal(__, lb = [lb_dd,lb_bg,lb_ex], ub = [ub_dd,ub_bg,ub_ex])

Since one seldom needs to re-define all boundaries for all subsets, it would make sense to simplify this to multiple keywords

fitsignal(__, lb_dd = lb_dd, lb_bg = lb_bg lb_ex = lb_ex)

this way a quick assignment can be done without needing to worry about the order of the parameter subsets.

@luisfabib luisfabib added this to the 0.12.0 milestone Oct 19, 2020
@luisfabib luisfabib self-assigned this Oct 19, 2020
@stestoll
Copy link
Collaborator

This sounds reasonable, and is consistent with the existing interface where the three models are already provided separately. To be completely consistent, this must be extended to the starting parameter par0 -> par0_dd, par0_bg, par0_ex. So the choice is between two extremes:

  • model, par0, lb, ub
  • model_dd, model_bg, model_ex, par0_dd, par0_bg, par0_ex, lb_dd, lb_bg, lb_ex, ub_dd, ub_bg, ub_ex

The second approach has the downside that it has a large number of keyword arguments. Maybe there is another way of organizing all these inputs. Group par0, lb, and ub for each model together? Or for each parameter?

@luisfabib luisfabib modified the milestones: 0.12.0, 0.13.0 Oct 23, 2020
@luisfabib
Copy link
Collaborator Author

After some consideration it seems that the second approach will be the most pythonian way of dealing with this. Having multiple keyword arguments with explicit names instead of a single keyword with nested lists and implicit ordering seems to align very well with the PEP20 guideline.

This will make the function call much readable only at the cost of more keywords to document. A PR will follow soon.

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

Successfully merging a pull request may close this issue.

2 participants