Skip to content

CLI and refactoring utils#8

Merged
dwhswenson merged 15 commits intoomsf:mainfrom
dwhswenson:cli
Apr 8, 2025
Merged

CLI and refactoring utils#8
dwhswenson merged 15 commits intoomsf:mainfrom
dwhswenson:cli

Conversation

@dwhswenson
Copy link
Copy Markdown
Member

Mostly this is the CLI. Aspects of the CLI are split into a few files:

  • cli.py: parses CLI input
  • main.py: runs the main function: gets a set of releases and filters them, providing the pkg_info dict.
  • output.py: various ways of converting the pkg_info dict to on-screen output.

This also involved moving some utilities under utils/dates.py and utils/packaging.py.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.18%. Comparing base (a33fb90) to head (9ee8175).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   98.76%   99.18%   +0.42%     
==========================================
  Files           7       15       +8     
  Lines         404      616     +212     
==========================================
+ Hits          399      611     +212     
  Misses          5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dwhswenson dwhswenson requested a review from Copilot April 3, 2025 04:24
Copy link
Copy Markdown
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

This PR introduces a new CLI and refactors utility functions for packaging and dates while updating tests accordingly. Key changes include the addition of a CLI interface in cli.py, the migration of date‐related functions to utils/dates.py, and updates to tests and logging to support the new structure.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/utils/test_packaging.py Adds tests for packaging utilities including specifier creation
tests/utils/test_dates.py Adds tests for date utility functions (quarter calculations, etc.)
tests/test_releasefilters.py Removes redundant date tests after refactoring out date functions
tests/test_output.py Introduces tests for JSON, specifier, and terminal output formats
tests/test_main.py Adds tests for main functionality and source/filter selection
src/spec0/utils/packaging.py Refactors packaging utilities and specifier generation
src/spec0/utils/dates.py Adds date utility functions for quarter handling and month shifting
src/spec0/releasesource.py Introduces logging during the fetching and processing of releases
src/spec0/releasefilters.py Refactors release filter to import date utilities instead of duplicating
src/spec0/output.py Implements various output formatting functions (JSON, specifier, table)
src/spec0/main.py Updates main function to integrate new sources, filters, and logging
src/spec0/cli.py Implements CLI parsing and selection of sources, filters, and outputs
Comments suppressed due to low confidence (1)

src/spec0/utils/packaging.py:11

  • [nitpick] The construction of the upper bound version always includes the epoch, which may be confusing when the epoch is 0. Consider omitting the epoch in the upper bound version for clarity to better match the expected output format.
upper = Version(f"{max_version.epoch}!{max_version.major + 1}.0")

Comment thread src/spec0/utils/dates.py Outdated
@dwhswenson dwhswenson requested a review from Copilot April 3, 2025 15:19
Copy link
Copy Markdown
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

This PR refactors and extends the CLI along with various utility functions related to packaging and dates while reorganizing tests accordingly. Key changes include:

  • New CLI implementation and option parsing in src/spec0/cli.py.
  • Refactoring and relocation of date and packaging utility functions.
  • Expanded test coverage for packaging, dates, main, and output functionality.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/utils/test_packaging.py Added tests for packaging utilities and specifier generation.
tests/utils/test_dates.py Added tests for date utilities with parameterized inputs.
tests/test_releasefilters.py Removed duplicate date tests previously in this file.
tests/test_output.py Added tests for JSON, specifier, and terminal output; checks output format.
tests/test_main.py Added tests for main function integration and release source selection.
src/spec0/utils/packaging.py Introduced maker functions for version specifiers and formatting.
src/spec0/utils/dates.py Added helper functions for quarter calculations and date shifting.
src/spec0/releasesource.py Enhanced logging around release fetching from PyPI.
src/spec0/releasefilters.py Cleaned up duplicate date functions and added logging in filtering.
src/spec0/output.py Implemented multiple output formats including JSON and terminal tables.
src/spec0/main.py Consolidated the main entry point with default source and filter usage.
src/spec0/cli.py Implemented the full CLI parser and execution flow for spec0.
Comments suppressed due to low confidence (1)

tests/utils/test_dates.py:7

  • [nitpick] The parameter name 'datetime' shadows the imported datetime module. Consider renaming it (e.g., 'dt') to avoid confusion.
@pytest.mark.parametrize("datetime, expected",

Comment thread tests/test_output.py
@dwhswenson dwhswenson requested a review from Copilot April 3, 2025 15:25
Copy link
Copy Markdown
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

This PR introduces a new CLI for spec0 and refactors utilities by separating packaging and date-related functions into their respective modules. Key changes include new and improved tests for utilities and outputs, enhanced logging in release source and release filters, and the implementation of a new CLI that supports multiple sources and output formats.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/utils/test_packaging.py New tests for ensuring correct version specifiers and formatting.
tests/utils/test_dates.py Added tests covering quarter and date shifting utilities.
tests/test_releasefilters.py Removed duplicate date tests.
tests/test_output.py Introduced tests for JSON, specifier, and terminal table outputs.
tests/test_main.py Added tests for main function behavior with dummy sources/filters.
src/spec0/utils/packaging.py Added packaging utilities including specifier generation and formatting.
src/spec0/utils/dates.py Added date utilities for quarter calculations and date shifting.
src/spec0/releasesource.py Enhanced logging for debugging of release fetching.
src/spec0/releasefilters.py Refactored filter module to remove duplicate functions and add logging.
src/spec0/output.py New module for outputting package information in various formats.
src/spec0/main.py New main module to integrate sources, filters, and output.
src/spec0/cli.py New CLI implementation with comprehensive argument parsing.
Comments suppressed due to low confidence (1)

src/spec0/utils/packaging.py:17

  • [nitpick] The local variable 'major_minor_str' inside the function 'major_minor_str' shadows the function name. Consider renaming it to 'result' or 'formatted_version' for clarity.
    major_minor_str = f"{version.major}.{version.minor}"

Comment thread src/spec0/cli.py Outdated
@dwhswenson dwhswenson requested a review from Copilot April 3, 2025 15:36
Copy link
Copy Markdown
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

This PR introduces a command-line interface for spec0 while refactoring utility functions into dedicated modules.

  • Added a CLI in src/spec0/cli.py with comprehensive options for selecting release sources, filters, and output formats.
  • Moved date and packaging utilities to src/spec0/utils with corresponding tests in tests/utils.
  • Updated and consolidated tests to ensure the new structure functions correctly.

Reviewed Changes

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

Show a summary per file
File Description
tests/utils/test_packaging.py Added tests for packaging utilities and version specifier logic.
tests/utils/test_dates.py Added tests for date utility functions like quarter conversion and month shifting.
tests/test_releasefilters.py Removed dated tests now covered by tests/utils/test_dates.py.
tests/test_output.py, tests/test_main.py Added tests for output generation and main functionality.
src/spec0/utils/packaging.py Introduced packaging utility functions for specifier creation and formatting.
src/spec0/utils/dates.py Added modular date utilities with robust edge-case handling.
src/spec0/releasesource.py Updated release source with added logging for debugging.
src/spec0/releasefilters.py Refactored to use external date utilities and added logging.
src/spec0/output.py Implemented various output formats including JSON and terminal output.
src/spec0/main.py Updated main function to integrate new sources and filters.
src/spec0/cli.py Developed a full-featured CLI with granular control over source, filter, and output selection.
Comments suppressed due to low confidence (1)

src/spec0/utils/packaging.py:17

  • [nitpick] The local variable 'major_minor_str' inside the function major_minor_str shadows the function name, which may lead to confusion. Consider renaming the variable (e.g., to 'result' or 'version_str').
major_minor_str = f"{version.major}.{version.minor}"

@dwhswenson dwhswenson requested a review from ethanholz April 3, 2025 15:37
@dwhswenson
Copy link
Copy Markdown
Member Author

@ethanholz : Went through a few cycles of Copilot review before asking for you review on this; my first time experimenting with that feature. FWIW, it caught a legit logic error, but then went pretty nitpicky. Low cost to use, with some positive impact. Worth using, IMO.

@ethanholz
Copy link
Copy Markdown
Contributor

Looks like it did a decent job, might be worth adding some policy around this when doing reviews on our code. Going to pull down this code and actually try it out before creating an approval. Files look good but want to validate user experience.

@dwhswenson
Copy link
Copy Markdown
Member Author

@ethanholz Entry point added!

@ethanholz
Copy link
Copy Markdown
Contributor

Seems to have a few issues in running some common examples I might use. For example:

This works

spec0 --pypi gha_runner

But this doesn't and fails with the accompanying error:

spec0 --conda-channel conda-forge openff-toolkit
Traceback (most recent call last):
  File "/Users/ethan/Documents/work/omsf-eco-infra/spec0/.venv/bin/spec0", line 10, in <module>
    sys.exit(cli_main())
             ^^^^^^^^^^
  File "/Users/ethan/Documents/work/omsf-eco-infra/spec0/src/spec0/cli.py", line 188, in cli_main
    sources = select_source(opts)
              ^^^^^^^^^^^^^^^^^^^
  File "/Users/ethan/Documents/work/omsf-eco-infra/spec0/src/spec0/cli.py", line 130, in select_source
    source = [CondaReleaseSource(platforms)]
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ethan/Documents/work/omsf-eco-infra/spec0/src/spec0/releasesource.py", line 109, in __init__
    data = json.load(f)
           ^^^^^^^^^^^^
  File "/Users/ethan/Library/Application Support/uv/python/cpython-3.12.4-macos-aarch64-none/lib/python3.12/json/__init__.py", line 293, in load
    return loads(fp.read(),
           ^^^^^^^^^^^^^^^^
  File "/Users/ethan/Library/Application Support/uv/python/cpython-3.12.4-macos-aarch64-none/lib/python3.12/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ethan/Library/Application Support/uv/python/cpython-3.12.4-macos-aarch64-none/lib/python3.12/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ethan/Library/Application Support/uv/python/cpython-3.12.4-macos-aarch64-none/lib/python3.12/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
               ^^^^^^^^^^^^^^^^^^^^^^
json.decoder.JSONDecodeError: Unterminated string starting at: line 1 column 180828498 (char 180828497)

Since we are at a point of having some actual usability for people, it might be helpful to have some example runs so that we can better test and validate the experience for our users.

@dwhswenson
Copy link
Copy Markdown
Member Author

@ethanholz : I can't reproduce that error. That looks like it is a problem with the cached version of the repodata.json. I was planning to do a little work on the caching, but I'd put that in another PR. FWIW, I get the following:

$ spec0 --conda-channel conda-forge openff-toolkit
Package             | Release Date | Drop Date
-----------------------------------------------
openff-toolkit 0.16 | 2024-04-12   | 2026-04-12
openff-toolkit 0.15 | 2024-01-26   | 2026-01-26
openff-toolkit 0.14 | 2023-06-30   | 2025-06-30
openff-toolkit 0.13 | 2023-04-26   | 2025-04-26

@ethanholz
Copy link
Copy Markdown
Contributor

Okay. We will have to evaluate more in the future.

Last question @dwhswenson , is there a reason that packaging isn't listed as a dependency in the pyproject.toml? Feel free to merge after.

@dwhswenson
Copy link
Copy Markdown
Member Author

is there a reason that packaging isn't listed as a dependency in the pyproject.toml?

Probably because I pick it up automatically with dev developments? (If nothing else, my local env also has Jupyter, which requires it.) Fixed in 9ee8175, merging!

@dwhswenson dwhswenson merged commit c267b53 into omsf:main Apr 8, 2025
5 checks passed
@dwhswenson dwhswenson deleted the cli branch April 8, 2025 19:49
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.

3 participants