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

Reconsider default arguments for low-level functions #116

Closed
ptmerz opened this issue Apr 20, 2021 · 0 comments · Fixed by #151
Closed

Reconsider default arguments for low-level functions #116

ptmerz opened this issue Apr 20, 2021 · 0 comments · Fixed by #151
Assignees
Milestone

Comments

@ptmerz
Copy link
Member

ptmerz commented Apr 20, 2021

It's worth considering removing most of the default arguments for low-level functions. Having defaults increases the chances to miss a usage when changing defaults or adding / removing arguments.

We should keep reasonable defaults for high-level (user-facing) functions.

@ptmerz ptmerz self-assigned this Apr 20, 2021
@ptmerz ptmerz added this to the JOSS version milestone Apr 29, 2021
ptmerz added a commit that referenced this issue May 9, 2021
Introduce typing hints throughout the code
Remove default values in the non-user facing functions (closes #116)
Fix a bug using the default kB for equipartition in all cases

* Introduce type hints and remove low-level default arguments for kinetic energy module
* Equipartition was using default kB in all cases. This was fixed, now adapt regression test values.
* Introduce type hints and remove low-level default arguments for ensemble module
* Introduce type hints and remove low-level default arguments for integrator module
* Add remaining type hints
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

Successfully merging a pull request may close this issue.

1 participant