Skip to content

Conversation

@stuartcampbell
Copy link
Collaborator

Hopefully these will provide a better and more reliable method of providing the version information.

This will now generate a file to contain the version _version.py and fallback to rely on the metadata only if this doesn't exist.

Copy link
Contributor

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

Introduce a more reliable versioning system by generating a _version.py file at build time and providing clear fallbacks, while logging and exposing the package version.

  • Read version from generated _version.py with metadata and default fallbacks
  • Log the package version on startup and set __version__ in the package init
  • Configure the hatch build hook, update Docker build steps, and ignore the virtual environment

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/nsls2api/version.py Add generated _version.py import with fallback
src/nsls2api/infrastructure/app_setup.py Log package version during application startup
src/nsls2api/init.py Expose __version__ using get_version()
pyproject.toml Configure hatch build hook for _version.py
Dockerfile Add build step before installing the package
.dockerignore Ignore .venv in Docker builds
Comments suppressed due to low confidence (5)

src/nsls2api/version.py:13

  • It looks like importlib isn’t imported in this module, which would cause a NameError. Add import importlib or import importlib.metadata at the top.
return importlib.metadata.version("nsls2api")

src/nsls2api/infrastructure/app_setup.py:9

  • background_service is imported but not used in this file; consider removing the unused import to clean up dependencies.
from nsls2api.services import background_service

src/nsls2api/version.py:5

  • Add unit tests for get_version() covering all paths: reading from _version.py, falling back to metadata, and the default "0.0.0" case to ensure version logic remains reliable.
def get_version() -> str:

src/nsls2api/version.py:15

  • [nitpick] The hard-coded default version "0.0.0" is a magic value; consider documenting why this specific fallback is used or reverting to "unknown" to clearly indicate missing version information.
return "0.0.0"  # Fallback version if the package is not found

src/nsls2api/init.py:4

  • [nitpick] Avoid calling get_version() at import time, as it may introduce side effects or import errors; consider loading the version lazily or using a static string.
__version__ = get_version()

Copy link
Collaborator

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

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

Good idea!

@padraic-shafer padraic-shafer merged commit 3ac17b1 into NSLS2:main Jun 9, 2025
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