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.

Sign up for GitHub

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

Jump to bottom

Is inv_triangle_number intended to be public? #509

Closed
paddyroddy opened this issue Feb 13, 2025 · 3 comments · Fixed by #527
Closed

Is inv_triangle_number intended to be public? #509

paddyroddy opened this issue Feb 13, 2025 · 3 comments · Fixed by #527
Assignees
@ntessore @Saransh-cpp
Labels
bug Something isn't working
Milestone
v2025.1

Comments

@paddyroddy
Copy link
Member

Describe the Bug

Currently to use this one needs to do glass.fields.inv_triangle_number rather than glass.inv_triangle_number. Should this be put in the top level __init__.py __all__ block? Otherwise should we mark it private?

To Reproduce

import glass
...

Expected Behaviour

No response

Actual Behaviour

No response

Version In Use

2024.3.dev48+gac7ca80

Additional Context

- Python version:
- Operating system:
@paddyroddy paddyroddy added the bug Something isn't working label Feb 13, 2025
@ntessore
Copy link
Collaborator

It's a trivial function, I don't think we need to make it public. But isn't it implicitly private by not being exported in the glass namespace?

@paddyroddy
Copy link
Member Author

Yes I guess so, but without the leading _ less clear that it's private

@ntessore
Copy link
Collaborator

Maybe we should rename this function to nfields_from_nspectra() or similar and make that public?

@Saransh-cpp Saransh-cpp assigned Saransh-cpp and ntessore and unassigned ntessore Feb 19, 2025
Saransh-cpp added a commit that referenced this issue Feb 19, 2025
@Saransh-cpp
gh-509: inv_triangle_number -> nfields_from_nspectra
6dcdf00
@Saransh-cpp Saransh-cpp mentioned this issue Feb 19, 2025
Merged
5 tasks
paddyroddy added a commit that referenced this issue Feb 19, 2025
@paddyroddy
gh-509: rename nfields_from_nspectra and make it public
9d93d31
@Saransh-cpp Saransh-cpp added this to the v2025.1 milestone Feb 19, 2025
Saransh-cpp added a commit that referenced this issue Feb 21, 2025
@Saransh-cpp @ntessore
gh-509: inv_triangle_number -> nfields_from_nspectra (plus make i…
1b407ea 
…t public) (#527)

Co-authored-by: Nicolas Tessore <n.tessore@ucl.ac.uk>
Saransh-cpp added a commit that referenced this issue Feb 21, 2025
@Saransh-cpp
gh-509: new inv_triangle_number popped up in another PR
6e7453e
@Saransh-cpp Saransh-cpp mentioned this issue Feb 21, 2025
Merged
5 tasks
paddyroddy pushed a commit that referenced this issue Feb 21, 2025
@Saransh-cpp
gh-509: new inv_triangle_number popped up in another PR (#528)
c7635f8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ntessore ntessore

@Saransh-cpp Saransh-cpp

Labels
bug Something isn't working
Projects
None yet
Milestone
v2025.1
Development

Successfully merging a pull request may close this issue.

gh-509: inv_triangle_number -> nfields_from_nspectra (plus make it public) glass-dev/glass
3 participants
@ntessore @paddyroddy @Saransh-cpp