Skip to content

test: add unit tests for relative_import - #5663

Merged
camdecoster merged 2 commits into
plotly:mainfrom
eeshsaxena:test/importers
Aug 1, 2026
Merged

test: add unit tests for relative_import#5663
camdecoster merged 2 commits into
plotly:mainfrom
eeshsaxena:test/importers

Conversation

@eeshsaxena

Copy link
Copy Markdown
Contributor

Description

_plotly_utils.importers.relative_import is the helper behind plotly's lazy
submodule imports - it returns the (__all__, __getattr__, __dir__) triple a
package assigns to enable PEP 562 lazy loading. It had no test coverage.

This adds tests/test_core/test_utils/test_importers.py covering:

  • __all__ listing the leaf names of the requested modules and classes
  • __getattr__ lazily importing a submodule
  • __getattr__ lazily importing a class off a submodule
  • __getattr__ raising AttributeError for an unknown name
  • __dir__ returning __all__

The tests use the stdlib collections package as a stable lazy-import target,
so they don't depend on plotly's own module layout. No source changes.

Verification

pytest tests/test_core/test_utils/test_importers.py - 5 passed. ruff check
/ ruff format --check clean.

@camdecoster

Copy link
Copy Markdown
Contributor

Thanks for the PR! I'll review it and follow up.

@camdecoster camdecoster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Before I merge, could you please move the file to tests/test_plotly_utils? That seems more appropriate.

Address review feedback: relocate the importers unit tests from
tests/test_core/test_utils/ to tests/test_plotly_utils/, which is the
more appropriate location for _plotly_utils tests. No test changes; the
import is absolute, so the move is behavior-preserving.
@eeshsaxena

Copy link
Copy Markdown
Contributor Author

Done, moved it to tests/test_plotly_utils/. Thanks for the review!

@eeshsaxena

Copy link
Copy Markdown
Contributor Author

Hi @camdecoster, gentle follow-up - the file's been moved to tests/test_plotly_utils/ as requested and CI is green. Happy to make any further tweaks; let me know if this is good to merge. Thanks!

@camdecoster
camdecoster merged commit b19eeb4 into plotly:main Aug 1, 2026
4 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.

2 participants