Skip to content

Conversation

@ajpotts
Copy link
Contributor

@ajpotts ajpotts commented May 19, 2025

This PR adds __all__ declarations to all modules where it was missing. This also removes import * statements from the project.

Closes #4426 add missing all

@ajpotts ajpotts force-pushed the 4426_add_missing_all branch from 6ea69c1 to 525e9d7 Compare May 19, 2025 21:04
@ajpotts ajpotts marked this pull request as ready for review May 19, 2025 21:06
@ajpotts ajpotts marked this pull request as draft May 19, 2025 22:41
@ajpotts ajpotts force-pushed the 4426_add_missing_all branch 2 times, most recently from 50c6172 to 439bcb2 Compare May 20, 2025 21:42
@ajpotts ajpotts marked this pull request as ready for review May 20, 2025 21:43
@ajpotts ajpotts force-pushed the 4426_add_missing_all branch from 439bcb2 to e7c19ed Compare May 20, 2025 21:51
Copy link
Contributor

@drculhane drculhane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passes all the tests. Looks good.

Copy link
Contributor

@1RyanK 1RyanK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the arkouda-util script, is this replacing * with all of the things in __all__? I feel like it would be nice if we could limit to only the things used, but maybe that's a little idealistic of me. The tradeoff is that it would make things maybe difficult to check to see if they're imported and to maintain the list of imported things. I guess for things like init.py you probably don't want to do that anyways...?

@ajpotts ajpotts enabled auto-merge May 21, 2025 18:10
Copy link
Contributor

@jaketrookman jaketrookman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@ajpotts ajpotts added this pull request to the merge queue May 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 21, 2025
@ajpotts ajpotts merged commit 4d54e62 into Bears-R-Us:master May 21, 2025
27 checks passed
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 this pull request may close these issues.

add missing __all__

4 participants