-
Notifications
You must be signed in to change notification settings - Fork 9
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
gh-509: inv_triangle_number
-> nfields_from_nspectra
(plus make it public)
#527
Conversation
Lol I was literally just about to raise a PR |
glass/fields.py
Outdated
r""" | ||
The :math:`n`-th triangle number is :math:`T_n = n \, (n+1)/2`. If | ||
the argument is :math:`T_n`, then :math:`n` is returned. Otherwise, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would update the argument and docstring here to reflect the new function name. Something along the lines of
Returns the number of fields for a number of spectra.
Given the number of spectra nspectra, returns the number of fields n such that
n * (n + 1) // 2 == nspectra
.
In addition, I would also update the error message:
invalid number of spectra: {nspectra}
That means there's no need to catch and rethrow the error elsewhere in fields.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated!
Co-authored-by: Nicolas Tessore <n.tessore@ucl.ac.uk>
Thanks for the fixes, @ntessore! |
Description
Renamed
inv_triangle_number
->nfields_from_nspectra
Made the function public and added it to
__all__
.Closes: #509
Changelog entry
Checks