Skip to content

feat: expose neuroglancer python exports via __all__ for public API#124

Draft
seankmartin wants to merge 1 commit intomasterfrom
feat/python-expose-exports-via-all
Draft

feat: expose neuroglancer python exports via __all__ for public API#124
seankmartin wants to merge 1 commit intomasterfrom
feat/python-expose-exports-via-all

Conversation

@seankmartin
Copy link

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the neuroglancer Python package’s top-level public API by explicitly defining __all__ and forwarding exports from viewer_state / viewer_config_state via lazy attribute access.

Changes:

  • Add a curated neuroglancer.__all__ list for the top-level package exports.
  • Re-export viewer_config_state and viewer_state symbols by extending __all__ from their module-level __all__.
  • Implement module __getattr__ to lazily resolve forwarded exports from viewer_config_state / viewer_state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +73 to +76
return getattr(viewer_config_state, name)
if name in viewer_state.__all__:
return getattr(viewer_state, name)
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

__getattr__ will re-resolve the same symbols on every access and those names won’t appear in dir(neuroglancer). Consider caching the resolved attribute into globals() before returning, and adding a __dir__ that includes the forwarded exports, to improve performance and introspection behavior.

Suggested change
return getattr(viewer_config_state, name)
if name in viewer_state.__all__:
return getattr(viewer_state, name)
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
value = getattr(viewer_config_state, name)
globals()[name] = value
return value
if name in viewer_state.__all__:
value = getattr(viewer_state, name)
globals()[name] = value
return value
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
def __dir__():
"""
Return the list of attributes available on the neuroglancer module,
including forwarded exports from viewer_config_state and viewer_state.
"""
return sorted(set(globals().keys()) | set(__all__))

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +67
__all__ = [
# Submodules
"segment_colors",
"server",
"skeleton",
# From default_credentials_manager
"set_boss_token",
# From equivalence_map
"EquivalenceMap",
# From local_volume
"LocalVolume",
# From screenshot
"ScreenshotSaver",
# From server
"is_server_running",
"set_server_bind_address",
"set_static_content_source",
"set_dev_server_content_source",
"stop",
# From url_state
"parse_url",
"to_json_dump",
"to_url",
# From viewer
"UnsynchronizedViewer",
"Viewer",
]

# Add exports from viewer_config_state and viewer_state
__all__ += viewer_config_state.__all__
__all__ += viewer_state.__all__
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This change introduces a public __all__ contract for the package. There are existing Python tests, but none appear to validate neuroglancer.__all__ itself; adding a small test that asserts key public names are present in __all__ and that every name in __all__ is actually importable/accessible would help prevent accidental API regressions.

Copilot uses AI. Check for mistakes.
Comment on lines 16 to +24
from . import ( # noqa: I001
segment_colors, # noqa: F401
server, # noqa: F401
skeleton, # noqa: F401
segment_colors,
server,
skeleton,
)
from .default_credentials_manager import set_boss_token # noqa: F401
from .equivalence_map import EquivalenceMap # noqa: F401
from .local_volume import LocalVolume # noqa: F401
from .screenshot import ScreenshotSaver # noqa: F401
from .default_credentials_manager import set_boss_token
from .equivalence_map import EquivalenceMap
from .local_volume import LocalVolume
from .screenshot import ScreenshotSaver
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

These imports are now only used for re-exporting, but Ruff has F (pyflakes) enabled and will flag them as unused (F401) because they aren’t referenced in this module. Add # noqa: F401 for the intended re-exports (or a file-level # ruff: noqa: F401) so linting doesn’t fail.

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +67
__all__ += viewer_config_state.__all__
__all__ += viewer_state.__all__
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

__all__ is extended from both viewer_config_state.__all__ and viewer_state.__all__, which includes overlapping names (e.g. LayerSelectedValues, LayerSelectionState, SegmentIdMapEntry). Consider de-duplicating __all__ (while preserving order) to avoid repeated entries and redundant work for from neuroglancer import *.

Suggested change
__all__ += viewer_config_state.__all__
__all__ += viewer_state.__all__
for _name in viewer_config_state.__all__:
if _name not in __all__:
__all__.append(_name)
for _name in viewer_state.__all__:
if _name not in __all__:
__all__.append(_name)

Copilot uses AI. Check for mistakes.
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