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

Specify that __all__ should be a tuple #632

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

parejkoj
Copy link
Contributor

We've had some conversation about this on slack in the past, and settled on tuple, but never specified it in the dev guide.

@parejkoj parejkoj requested a review from timj July 18, 2023 23:52
@timj
Copy link
Member

timj commented Jul 19, 2023

A tuple seems right to me given that you don't want someone to change the content, but there is a problem that the Python docs themselves use a list: https://docs.python.org/3/tutorial/modules.html#importing-from-a-package

Not sure if we want to require tuple or just allow Sequence.

@timj
Copy link
Member

timj commented Jul 19, 2023

The other downside of tuple is that:

__all__ = ("myfunc")

is not what you want but

__all__ = ["myfunc"]

will work without having to worry about the trailing comma.

@parejkoj
Copy link
Contributor Author

Yeah, there's a tradeoff either way. Having ("myfunc") is caught immediately on import, and we don't have a lot of single-object modules. Should we have a poll?

@timj
Copy link
Member

timj commented Jul 24, 2023

I'm happy to let @ktlim decide.

@ktlim
Copy link
Contributor

ktlim commented Aug 22, 2023

It's really hard to search for __all__ tuple list on Slack :)

A GitHub search for __all__ = ( gives 234K code results; __all__ = [ gives 963K code results.

The "caught immediately on import" for a missing trailing comma gives a less-than-fully-helpful error (AttributeError: module 'y' has no attribute 'm').

https://stackoverflow.com/questions/66098828/using-list-instead-of-tuple-in-module-all gives some more arguments for list over tuple (including that it is sometimes useful to extend a list programmatically). "Accidentally" modifying __all__ seems pretty unlikely.

I would prefer to go with the syntax in the Python docs, so until the docs give a tuple example, we should stick with list.

@ktlim
Copy link
Contributor

ktlim commented Aug 22, 2023

Note that even cpython is internally inconsistent: the somewhat newer asyncio and tomllib use tuple, while most other modules seem to use list (including ones where it's required due to modification).

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.

None yet

3 participants