ci: migrate to dynamic version with setuptools-scm, include version in serialized inputs/outputs #17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n serialized inputs/outputs
Summary of ChangesHello @anmorgunov, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Pytest Coverage Report 🧪 |
anmorgunov
added
scope/ci-cd
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the project to dynamic versioning using setuptools-scm and includes the package version in serialized data structures for improved reproducibility. The changes are logical and well-implemented across pyproject.toml, uv.lock, and the source code. I have a couple of suggestions to optimize the version retrieval during serialization by caching it at module load time, which will improve performance if these methods are called frequently. Please also consider that the tests for serialization will likely need updates to account for the new calcflow_version field in the output dictionaries.
src/calcflow/common/input.py
Outdated
| """ | ||
| data = asdict(self) | ||
| # asdict already recursively converts nested dataclasses | ||
| data["calcflow_version"] = version("calcflow") |
There was a problem hiding this comment.
Calling version("calcflow") on every serialization can introduce performance overhead, as it may involve filesystem access to read package metadata. Since the version is constant for a given installation, it's more efficient to query it once when the module is loaded and cache the result.
Consider this pattern:
# At the top of the file, after imports
_CALCFLOW_VERSION = version("calcflow")
# ...
# In the to_dict method
data["calcflow_version"] = _CALCFLOW_VERSIONThis change will prevent repeated lookups if to_dict() is called in a hot loop. A similar improvement can be made in src/calcflow/common/results.py.
src/calcflow/common/results.py
Outdated
| Includes calcflow_version for tracking which version created this result.""" | ||
| data = super().to_dict() | ||
| data.pop("raw_output", None) | ||
| data["calcflow_version"] = get_version("calcflow") |
There was a problem hiding this comment.
To improve performance, the package version should be fetched only once at module load time instead of on every call to to_dict(). This avoids the potential overhead of repeated filesystem access from get_version().
You can cache the version in a module-level constant:
# At the top of the file, after imports
_CALCFLOW_VERSION = get_version("calcflow")
# ...
# Then, inside the to_dict method
data["calcflow_version"] = _CALCFLOW_VERSION![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
github-actions
bot
removed
the
type/feature
Pytest Coverage Report 🧪 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Greptile Overview
Greptile Summary
Migrated from manual version management to
setuptools-scmfor automated git-tag-based versioning and added version tracking to serialized data structures.Key Changes:
pyproject.tomlafter releasessetuptools-scmto derive version dynamically from git tags at build time__version__attribute to package root usingimportlib.metadata.version()CalculationInput.to_dict()andCalculationResult.to_dict()now includecalcflow_versionfieldfrom_dict()methods properly stripcalcflow_versionfield before reconstructionBenefits:
Confidence Score: 5/5
Important Files Changed
File Analysis
__version__attribute using importlib.metadata for runtime version accessSequence Diagram
sequenceDiagram participant Dev as Developer participant GH as GitHub Release participant Workflow as Publish Workflow participant SCM as setuptools-scm participant Git as Git Tags participant PyPI as PyPI participant User as Package User Note over Dev,GH: Release Process Dev->>GH: Create and publish release with tag (e.g., v0.2.3) GH->>Workflow: Trigger on release.published event Note over Workflow,Git: Build Process Workflow->>Git: Checkout master with fetch-depth: 0 Workflow->>SCM: Build package with uv build SCM->>Git: Read all git tags and history Git-->>SCM: Return tags (v0.2.3) SCM->>SCM: Generate version from latest tag Note over Workflow,PyPI: Publish Workflow->>Workflow: Run smoke tests Workflow->>PyPI: Publish to PyPI Note over User: Usage & Serialization User->>User: Install package from PyPI User->>User: Import calcflow Note over User: __version__ = "0.2.3" User->>User: Create CalculationInput/Result User->>User: Call .to_dict() or .to_json() Note over User: Output includes "calcflow_version": "0.2.3" User->>User: Call .from_dict() or .from_json() Note over User: Version field removed, backward compatible