Skip to content

Conversation

@rororowyourboat
Copy link

Closes #656

Summary

This PR introduces uv as the recommended package manager for MSML, providing fast and reproducible dependency management.

Changes

  • ✅ Added for deterministic dependency resolution
  • ✅ Updated to for PyGithub compatibility
  • ✅ Created with comprehensive development setup instructions
  • ✅ Updated to reference the new setup guide
  • ✅ Added CI workflow for testing with uv across multiple Python versions and platforms

Benefits

  • Faster installs: uv is significantly faster than pip
  • Reproducible builds: ensures consistent environments
  • CI integration: Automated testing with uv in GitHub Actions
  • Better developer experience: Clear setup instructions for contributors

Testing

  • Manual testing: successfully installs all dependencies
  • CI workflow added to test across Python 3.8-3.12 and multiple OS platforms

- Add uv.lock for reproducible dependency resolution
- Update requires-python to >=3.8 for PyGithub compatibility
- Add SETUP.md with development environment instructions
- Update README.md with link to setup guide
- Add CI workflow for testing with uv across multiple platforms
- Addresses issue #656
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

This PR introduces uv as the recommended package manager for MSML, modernizing the dependency management approach with faster installs and reproducible builds.

Changes:

  • Updated minimum Python version requirement from 3.7 to 3.8 in pyproject.toml
  • Created comprehensive SETUP.md guide with uv installation and usage instructions
  • Added reference to SETUP.md in README.md for contributors
  • Introduced CI workflow for testing package installation with uv across Python 3.8-3.12 and multiple platforms

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

File Description
pyproject.toml Updates Python version requirement to >=3.8, aligning with modern Python support and uv compatibility
SETUP.md New comprehensive development setup guide with uv installation, dependency management, and workflow instructions
README.md Adds link to new SETUP.md for contributors seeking development setup guidance
.github/workflows/test.yml New CI workflow for automated testing of package installation across multiple Python versions and operating systems

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

run: uv sync --frozen

- name: Verify installation
run: uv run python -c "import math_spec_mapping; print(math_spec_mapping.__version__)"
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The verification step attempts to print math_spec_mapping.__version__, but the __init__.py file does not define a __version__ attribute. This will cause the workflow to fail. Either add a __version__ attribute to the package's __init__.py file, or change this verification to a simpler import check without accessing the version attribute.

Suggested change
run: uv run python -c "import math_spec_mapping; print(math_spec_mapping.__version__)"
run: uv run python -c "import math_spec_mapping"

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,33 @@
name: Test
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The workflow is named "Test" but only verifies that the package can be imported successfully. It doesn't run any actual test suite. Consider renaming this workflow to something more accurate like "Install and Verify" or "Build" to avoid confusion, or add actual test execution if test files exist or are planned.

Suggested change
name: Test
name: Install and Verify

Copilot uses AI. Check for mistakes.
run: uv python install ${{ matrix.python-version }}

- name: Install dependencies
run: uv sync --frozen
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The workflow installs Python with uv python install but doesn't explicitly configure uv to use that specific Python version for subsequent commands. Consider pinning the Python version by either: (1) using uv sync --python ${{ matrix.python-version }}, or (2) setting up the Python version using GitHub's actions/setup-python action before installing uv, which will ensure uv automatically detects and uses that Python version.

Suggested change
run: uv sync --frozen
run: uv sync --frozen --python ${{ matrix.python-version }}

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +85
# Add a development dependency
uv add --dev <package-name>
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The flag --dev for adding development dependencies may not be the correct syntax for uv. As of recent uv versions, development dependencies are typically added using --group dev or by specifying them in [project.optional-dependencies] or [tool.uv.dev-dependencies] sections in pyproject.toml. Please verify the correct syntax for the version of uv being used.

Suggested change
# Add a development dependency
uv add --dev <package-name>
# Add a development dependency (in the "dev" group)
uv add --group dev <package-name>

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

@copilot can you help solve

The failure is due to this line in src/math_spec_mapping/Load/type.py (ref: 187b270):

from typing import _UnionGenericAlias, List, _GenericAlias

The names _UnionGenericAlias and _GenericAlias are private members of the typing module and are not guaranteed to exist in all Python versions. In Python 3.8, these members areThe job failed due to ImportError: cannot import name '_UnionGenericAlias' from 'typing'. This occurs because _UnionGenericAlias and _GenericAlias are internal classes and are not part of the public API of the typing module. They are not guaranteed to exist across Python versions and their use is discouraged outside the standard library.

To fix this, replace any references to _UnionGenericAlias and _GenericAlias with public interfaces. For checking generic types like Union or List, you can use typing.get_origin and typing.get_args (available in Python 3.8+).

Update the relevant section in src/math_spec_mapping/Load/type.py (ref: 187b270) as follows:

from typing import List, get_origin

# Remove: from typing import _UnionGenericAlias, List, _GenericAlias

# In your logic where _UnionGenericAlias or _GenericAlias not directly importable, causing the ImportError.

To fix this:

1. Remove the import of _UnionGenericAlias and _GenericAlias from the import statement.
2. Use typing's public API to check for union and generic aliases. In Python 3.8+, the best practice is to use typing.get_origin and typing.get_args (Python 3.8+ with typing_extensions, otherwise Python 3.9+ natively).

Here's how you could change the logic:

```python
from are used:
if get_origin(data["type"]["python"]) is not None:
    data["type_name"]["python"] = data["type"]["python"].__repr__()
else:
    data["type_name"]["python"] = data["type"]["python"].__name__

This change uses get_origin to check for generic types instead of relying on internal classes. This will resolve the ImportError and make the code forward-compatible with modern Python versions.

Copy link

Copilot AI commented Jan 22, 2026

@rororowyourboat I've opened a new pull request, #661, to work on those changes. Once the pull request is ready, I'll request review from you.

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.

setting up uv based package management

2 participants