Skip to content

__all__ must be a sequence of strings: fix __init__.py and factory.py#247

Merged
rayrrr merged 2 commits into
jazzband:mainfrom
p-vdp:main
May 28, 2026
Merged

__all__ must be a sequence of strings: fix __init__.py and factory.py#247
rayrrr merged 2 commits into
jazzband:mainfrom
p-vdp:main

Conversation

@p-vdp
Copy link
Copy Markdown
Contributor

@p-vdp p-vdp commented May 20, 2026

See the documentation:

The public names defined by a module are determined by checking the module’s namespace for a variable named __all__; if defined, it must be a sequence of strings which are names defined or imported by that module.

pytest ./tests is passing.

@p-vdp p-vdp changed the title __all__ must be a sequence of strings, fix top level __init__.py __all__ must be a sequence of strings: fix __init__.py and factory.py May 20, 2026
Copy link
Copy Markdown
Member

@rayrrr rayrrr left a comment

Choose a reason for hiding this comment

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

Thanks @p-vdp for the contribution. However, given that #40 is a long-standing issue, my vote is to remove __all__ completely from both files instead. __all__ is optional to begin with, and only facilitates star imports, which are considered a bad practice.

Background: https://discuss.python.org/t/usage-of-all-in-init-py/17936

OK?

@p-vdp
Copy link
Copy Markdown
Contributor Author

p-vdp commented May 28, 2026

I opened this because passing objects through __all__ breaks external tooling that expects strings. If there's a larger refactor planned I understand.

@rayrrr
Copy link
Copy Markdown
Member

rayrrr commented May 28, 2026

@p-vdp we can make a change like this for now and remove it later, but please keep the ([] + [] + [] ...) syntax that was there in the first place to maintain readability. It will still be a a valid sequence.

@p-vdp p-vdp requested a review from rayrrr May 28, 2026 20:32
@rayrrr rayrrr merged commit 3fba3cd into jazzband:main May 28, 2026
8 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.61%. Comparing base (eed8089) to head (a43407f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   98.61%   98.61%           
=======================================
  Files          20       20           
  Lines         794      794           
=======================================
  Hits          783      783           
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants