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

Rename glass.core.array to glass.arraytools #478

Closed
paddyroddy opened this issue Jan 14, 2025 · 7 comments · Fixed by #524
Closed

Rename glass.core.array to glass.arraytools #478

paddyroddy opened this issue Jan 14, 2025 · 7 comments · Fixed by #524
Assignees
Labels
api An (incompatible) API change bug Something isn't working

Comments

@paddyroddy
Copy link
Member

Describe the Bug

Picked up by ruff==0.9.1 in #477. For now I have suppressed with # noqa: A005.

To Reproduce

import glass
...

Expected Behaviour

No response

Actual Behaviour

No response

Version In Use

2024.3.dev32+g0b41f16

Additional Context

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

Do we care?

@paddyroddy
Copy link
Member Author

@ntessore it's an up-to-you situation. It could cause unwanted issues https://docs.astral.sh/ruff/rules/stdlib-module-shadowing, but if you want to keep it then please close this issue.

@ntessore
Copy link
Collaborator

I realise that it could cause issues in general, but I can vouch for the fact that we are not going to use the stdlib array module 😄

@ntessore
Copy link
Collaborator

That said, if we don't want to override the rule on a case-by-case basis, we can also rename that module to something else. I really don't care!

@Saransh-cpp
Copy link
Member

This can definitely create problems. I'd suggest renaming the module to lib (numpy's broadcasting functions live in lib) or numeric (for trapezoidal rules). Other options would be to split the module into two modules, lib and numeric, or just keep all of it directly under the namespace of core (which sounds nice too - move all the functions to __init__.py_.

Let me know which one sounds good and I can create a PR!

@Saransh-cpp Saransh-cpp self-assigned this Feb 17, 2025
@ntessore
Copy link
Collaborator

If glass.core.algorithm becomes glass.algorithm (#513) then we could remove glass.core entirely, and make this glass.arraytools or whatever you prefer?

@Saransh-cpp
Copy link
Member

That makes sense. arraytools sounds much more better.

@Saransh-cpp Saransh-cpp changed the title A005 Module array shadows a Python standard-library module Rename glass.core.array to glass.arraytools Feb 17, 2025
@Saransh-cpp Saransh-cpp added the api An (incompatible) API change label Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api An (incompatible) API change bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants