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

Reenable test_all, fix _aliases.__all__ #286

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ev-br
Copy link
Member

@ev-br ev-br commented Mar 22, 2025

closes gh-283

  • reenable test_all
  • fix common._aliases.__all__ which was wrong
  • make it easier to run self-tests locally, esp if not all optional dependencies are available.

So the __all__ list was wrong for a while, but it was not caught be tests because:

  • locally, self-tests required all optional dependencies to be installed, so I never ran the whole $ pytest tests/ locally
  • on CI, the problem was not caught either, because the test iterates over sys.modules. So if an implementation module is not imported, it's not checked: this was the case with common._aliases until BUG: torch: fix result_type with python scalars #277 added an unrelated test.

@ev-br ev-br added this to the 1.12 milestone Mar 22, 2025
@ev-br ev-br requested a review from asmeurer March 22, 2025 13:13
@ev-br ev-br mentioned this pull request Mar 22, 2025
ev-br added a commit that referenced this pull request Mar 23, 2025
This reverts commit 5473d84.
@ev-br ev-br force-pushed the test_all branch 2 times, most recently from 0e339fc to 9bb1021 Compare March 23, 2025 09:07
@ev-br
Copy link
Member Author

ev-br commented Mar 28, 2025

I think I might want to merge this PR sooner than later, if only to restore the state of the tests and make the CI green by default. Please speak up if you want some more time to review.

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

thanks Evgeni! Just one comment

Co-authored-by: Evgeni Burovski <evgeny.burovskiy@gmail.com>
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.

skipped test_all
2 participants