Skip to content

Update type checking to use numpy NDArray and ArrayLike#133

Merged
jatkinson1000 merged 6 commits into
mainfrom
numpy-typing
Apr 28, 2025
Merged

Update type checking to use numpy NDArray and ArrayLike#133
jatkinson1000 merged 6 commits into
mainfrom
numpy-typing

Conversation

@jatkinson1000

@jatkinson1000 jatkinson1000 commented Apr 26, 2025

Copy link
Copy Markdown
Owner

Use numpy typing closer to intended use as suggested by @LiamPattinson in #130

Take inputs to functions as ArrayLike, casting using np.asarray() where appropriate.
Return items as NDArray[float64] with single values being duck-typed in the majority of cases.
Removes the FloatArray type to just use NDArray[float64] explicitly.

Resolves a number of issues seen in latest mypy in a more sensible manner than #130
Also allows us to simplify some of the "type-checker" appeasement throughout the code e.g. castings and overloads.

…numpy typing suggested practices.

Co-authored-by: Liam Pattinson <liampattinson@gmail.com>
@jatkinson1000
jatkinson1000 requested a review from Copilot April 27, 2025 17:07

Copilot AI left a comment

Copy link
Copy Markdown

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 updates the type checking for handicap functions by replacing the custom FloatArray type with standard numpy typings (ArrayLike and NDArray), and adjusts error messages to reflect these changes.

  • Replace and simplify type hints across several modules
  • Update error messages for type conversions
  • Remove the deprecated FloatArray type from the codebase

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
archeryutils/handicaps/tests/test_handicap_tables.py Updated expected error message for invalid handicap inputs
archeryutils/handicaps/handicap_tables.py Adjusted type hints (using npt.ArrayLike and NDArray[np.float64]) and streamlined input sanitization
archeryutils/handicaps/handicap_scheme_agb.py Updated type annotations to use npt.ArrayLike and NDArray[np.float64]
archeryutils/handicaps/handicap_scheme_aa.py Updated type annotations to use npt.ArrayLike and NDArray[np.float64]
archeryutils/handicaps/handicap_scheme.py Removed FloatArray alias and overload signatures; unified type hints with npt.ArrayLike
archeryutils/handicaps/handicap_functions.py Updated type hints to align with the revised numpy typing scheme
archeryutils/handicaps/init.py Removed FloatArray from the exported symbols
archeryutils/classifications/agb_old_indoor_classifications.py Converted hard-coded handicap lists to numpy arrays with explicit float64 type

@jatkinson1000
jatkinson1000 requested a review from Copilot April 27, 2025 17:19

Copilot AI left a comment

Copy link
Copy Markdown

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 updates the type annotations across the codebase to use numpy’s official typing constructs (npt.ArrayLike and npt.NDArray[np.float64]) in place of the custom FloatArray type. It also removes the FloatArray alias and updates function signatures, docstrings, and error messages accordingly.

  • Updated type hints in handicap schemes, tables, and functions
  • Removed the custom FloatArray alias for a more standardized approach
  • Revised error messages and documentation to reflect the changes

Reviewed Changes

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

Show a summary per file
File Description
archeryutils/handicaps/tests/test_handicap_tables.py Updated error message match string in tests to reflect new type-checking behavior
archeryutils/handicaps/handicap_tables.py Changed type annotations and output types for handicap tables and input sanitization
archeryutils/handicaps/handicap_scheme_agb.py Updated sigma_t function signature and added explicit numpy array conversion
archeryutils/handicaps/handicap_scheme_aa.py Updated sigma_t function signature and added explicit numpy array conversion
archeryutils/handicaps/handicap_scheme.py Removed custom FloatArray and updated all function signatures and return types
archeryutils/handicaps/handicap_functions.py Updated type annotations in helper functions to align with new numpy type hints
archeryutils/handicaps/init.py Removed FloatArray export to enforce standard types
archeryutils/classifications/agb_old_indoor_classifications.py Converted classification handicap lists to np.array for consistency
Files not reviewed (1)
  • docs/develop/whats-new.rst: Language not supported

@jatkinson1000

Copy link
Copy Markdown
Owner Author

How far should ArrayLike be allowed to propagate in the code?

ArrayLike includes a numpy array or list of strings, and we should check that inputs can be cast to numbers before too deep. Probably using np.can_cast() https://numpy.org/doc/stable/reference/generated/numpy.can_cast.html as:

if not numpy.can_cast(np.asarray(input), np.float64):
    Raise("Inappropriate input for code. Must be numeric value.")

Perhaps wrap this up as a reuseable function, and even use it to return the input cast to a np.float64 array?

np.float64.

Also catches a couple of documentation typos.
@jatkinson1000

Copy link
Copy Markdown
Owner Author

Decided to allow it to propagate all the way to the bottom (sigma_t) as this will catch all instances in one place, rather than being required in several functions. The backtrace will ensure that users can see where the error began/the calling function.

Only downside is that anyone who implements a new scheme should build this into their implementation of sigma_t as it is not guarded anywhere in the general handicap_scheme parent class. The alternative would be to perform the check in sigma_r and require that sigma_t takes NDArray[np.float64] as an input.

@LiamPattinson LiamPattinson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this all looks better now. Just have a few comments.


How far should ArrayLike be allowed to propagate in the code?

I think the way you've done it is sensible. The rules I generally follow are:

  • If you call any NumPy functions on some input, type it as ArrayLike and use np.asarray(x) at the top of the function.
  • If you only pass ArrayLike things to other functions taking ArrayLike, simply propagate it forward.
  • If something must be a NumPy array on entry, instead type it as npt.NDArray[np.float64] (or something similar).

Comment thread archeryutils/handicaps/handicap_scheme.py Outdated
Comment thread archeryutils/handicaps/handicap_tables.py Outdated
Comment thread archeryutils/handicaps/handicap_scheme.py Outdated
than using typing.cast

Co-authored-by: Liam Pattinson <liampattinson@gmail.com>

@LiamPattinson LiamPattinson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool, looks good to me 👍

@jatkinson1000
jatkinson1000 merged commit 77e8cfb into main Apr 28, 2025
@jatkinson1000
jatkinson1000 deleted the numpy-typing branch April 28, 2025 16:14
@codecov

codecov Bot commented Jul 27, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.94%. Comparing base (68ddf48) to head (954a89b).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   97.92%   97.94%   +0.01%     
==========================================
  Files          31       31              
  Lines        2028     2043      +15     
==========================================
+ Hits         1986     2001      +15     
  Misses         42       42              
Files with missing lines Coverage Δ
.../classifications/agb_old_indoor_classifications.py 100.00% <100.00%> (ø)
archeryutils/handicaps/__init__.py 100.00% <100.00%> (ø)
archeryutils/handicaps/handicap_functions.py 100.00% <100.00%> (ø)
archeryutils/handicaps/handicap_scheme.py 96.95% <100.00%> (-0.04%) ⬇️
archeryutils/handicaps/handicap_scheme_aa.py 100.00% <100.00%> (ø)
archeryutils/handicaps/handicap_scheme_agb.py 100.00% <100.00%> (ø)
archeryutils/handicaps/handicap_tables.py 90.10% <100.00%> (-0.32%) ⬇️
...cheryutils/handicaps/tests/test_handicap_tables.py 100.00% <ø> (ø)
archeryutils/handicaps/tests/test_handicaps.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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