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

Variable naming scheme? #2544

Open
jwreep opened this issue Feb 28, 2024 · 2 comments
Open

Variable naming scheme? #2544

jwreep opened this issue Feb 28, 2024 · 2 comments
Labels
contributor guide refactoring ♻️ Improving an implementation without adding new functionality

Comments

@jwreep
Copy link
Contributor

jwreep commented Feb 28, 2024

Description of improvement

I'm new to the package, so perhaps this may have been discussed in the past. When reading over the code in a few places to look at the suggested good first issues, one thing that strikes me is that the naming scheme for variables is, well, variable.

For example, in the gyroradius function:
lorentzfactor is spelled out, but neither in CamelCase nor separated by underscores.
T and B are fully abbreviated
Vperp is sort of in CamelCase, and partially abbreviated
nans_in_both_T_and_Vperp is separated by underscores
isfinite_lorentzfactor is partially separated by underscores, and partially not.

For legibility of the code, would it be worth considering adopting a single style for the variable names? Just a thought, but it came to mind when reading issue #1983.

@jwreep jwreep added the refactoring ♻️ Improving an implementation without adding new functionality label Feb 28, 2024
@namurphy
Copy link
Member

Thank you for raising this issue! We've made some progress on naming consistency in the last few years (in particular with particle inputs) but as you've noticed it's still a work in progress. We have some discussion of this in our coding guide, though it's still incomplete. Improving the consistency of names within functions (and making names more compliant with the PEP 8 style guide) might a good first issue for new contributors, though we'd need to be careful about backward compatibility when changing the names of arguments provided to a public-facing function.

With respect to gyroradius itself, I think we may have originally named lorentzfactor as one word to avoid a namespace conflict with plasmapy.formulary.relativity.lorentz_factor, and I suspect isfinite was probably inspired by numpy.isfinite.

Thank you again!

Cross-references: #176, #470, #706, #1412

@jwreep
Copy link
Contributor Author

jwreep commented Mar 5, 2024

Thanks for the helpful response. I see that the coding guide does address most of my concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor guide refactoring ♻️ Improving an implementation without adding new functionality
Projects
None yet
Development

No branches or pull requests

2 participants