Skip to content

Conversation

@paulz
Copy link
Contributor

@paulz paulz commented Mar 28, 2025

now CI will check both tests and src folders for type errors.

@paulz paulz requested a review from Copilot March 28, 2025 01:37
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

This PR fixes type issues in the tests and expands CI type checking to include both the src and tests folders.

  • Converts values for failure rate analysis to int in tests to satisfy type constraints.
  • Updates import paths in tests to align with the refactored module structure.
  • Adds a dependency on types-jsonschema and configures mypy to relax restrictions for tests.

Reviewed Changes

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

Show a summary per file
File Description
tests/test_statistical_analysis.py Converts failure and total values to int to resolve type errors.
tests/test_runner.py Updates import paths to the refactored package structure.
tests/test_reporter.py Adjusts import paths to match new module locations.
pyproject.toml Adds types-jsonschema dependency and mypy override for tests.
examples/team_recommender/src/response_matches_json_schema.py Improves type hinting by using typing.Any.
.github/workflows/python-tests.yml Extends mypy checking to include tests and renames linter job.
Comments suppressed due to low confidence (1)

tests/test_statistical_analysis.py:233

  • Ensure that all entries in 'failures' and 'totals' are guaranteed to be convertible to int to avoid potential ValueError exceptions during test execution.
analyse_failure_rate_from_test_sample(int(f), int(t))

carl added 2 commits March 27, 2025 18:45
in graph data calculation

Signed-off-by: Paul Zabelin <[email protected]>
@paulz paulz requested a review from Copilot March 28, 2025 01:47
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

This pull request addresses type errors by ensuring that both the tests and source folders are subject to type checking. The key changes include updating test data types, revising import paths to match the new package structure, and adjusting CI configurations for comprehensive type checking.

Reviewed Changes

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

Show a summary per file
File Description
tests/test_statistical_analysis.py Changed numeric data representations for consistency with type checking expectations
tests/test_runner.py Updated import paths to reflect project restructuring
tests/test_reporter.py Updated import paths for helper and reporter modules
pyproject.toml Added a dependency for types-jsonschema and defined mypy overrides for tests
examples/team_recommender/tests/example_1_text_response/test_good_fit_for_project.py Refactored assertion for improved clarity
examples/team_recommender/src/response_matches_json_schema.py Updated the type of the schema argument to use Any from typing
.github/workflows/python-tests.yml Expanded type checking to include tests in addition to source code
Comments suppressed due to low confidence (3)

tests/test_statistical_analysis.py:228

  • Switching from a numpy array (from np.ones(100)*100) to a list might affect downstream numerical operations if functions expect numpy arrays. Please verify that this change does not break any arithmetic or array-specific behaviors in analyse_failure_rate_from_test_sample.
totals = [100] * 100

tests/test_runner.py:1

  • Ensure that the updated import paths (e.g. from cat_ai.reporter) correctly reflect the new project structure and that all corresponding modules are accessible.
from cat_ai.reporter import Reporter

tests/test_reporter.py:5

  • Confirm that the new import path accurately points to the helpers module after restructuring, ensuring consistency across tests and production code.
from cat_ai.helpers.helpers import root_dir

@paulz paulz requested a review from Copilot March 28, 2025 18:17
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

This PR fixes type errors by expanding type checking to tests and source folders. Key changes include:

  • Updating test code to use Python lists instead of NumPy arrays.
  • Adjusting import paths in several test files.
  • Revising pyproject.toml for license declaration and adding type-check and lint dependency overrides.
  • Expanding the CI workflow to include additional folders in type checking and linting.

Reviewed Changes

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

Show a summary per file
File Description
tests/test_statistical_analysis.py Changed failure sample creation to use Python lists
tests/test_runner.py Updated import paths to reflect new module layout
tests/test_reporter.py Adjusted import order and paths
pyproject.toml Altered license format and added dependency/group changes
examples/team_recommender/tests/example_1_text_response/test_good_fit_for_project.py Modified assert logic for hallucination scoring
examples/team_recommender/tests/example_1_text_response/openai_embeddings.py Wrapped struct unpack result in float conversion
examples/team_recommender/src/response_matches_json_schema.py Updated type annotation for the schema parameter
.github/workflows/python-tests.yml Expanded type checking and linting to additional folders
Comments suppressed due to low confidence (2)

tests/test_statistical_analysis.py:228

  • Please verify that the function 'analyse_failure_rate_from_test_sample' handles Python lists correctly, as the previous implementation used NumPy arrays for potential vectorized operations.
totals = [100] * 100

pyproject.toml:19

  • Changing the license field from a dictionary to a string may affect tools expecting the earlier format; please confirm that downstream tooling supports this new declaration.
license = "MIT"

@kseebaldt kseebaldt merged commit c30cf06 into thisisartium:main Apr 2, 2025
1 check 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.

3 participants