Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python] Typeguard not running as intended #2238

Closed
ryan-williams opened this issue Mar 7, 2024 · 1 comment · Fixed by #1960
Closed

[python] Typeguard not running as intended #2238

ryan-williams opened this issue Mar 7, 2024 · 1 comment · Fixed by #1960
Assignees

Comments

@ryan-williams
Copy link
Member

ryan-williams commented Mar 7, 2024

#2239 has a repro of typeguard failing to raise on non-test code with intentionally broken types.

I believe the issue is, in this block:

# avoid typeguard by importing before calling install_import_hook
from tiledbsoma import _query_condition # noqa: F401
install_import_hook("tiledbsoma")

Line 5 is inadvertently importing ≈all of tiledbsoma (it executes tiledbsoma/__init__.py, which imports most things) before the typeguard hook is initialized, meaning ≈nothing is being checked by typeguard.

I suspect this has been the case since that line was added, in #496 (Nov '22).

Attempting to restore Typeguard checks

When I comment out the _query_condition line, typeguard raises in any pytest command that executes tiledbsoma/__init__.py, e.g.:

echo 'import tiledbsoma' > tests/test_typeguard.py
pytest tests/test_typeguard.py
# ERROR tests/test_typeguard.py - TypeError: Subscripted generics cannot be used with class and instance checks
Full stack trace
================================================== test session starts ==================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
rootdir: /Users/ryan/c/tiledb/tiledb-soma/apis/python
plugins: anyio-4.3.0, typeguard-2.13.3
collected 0 items / 1 error

======================================================== ERRORS =========================================================
_______________________________________ ERROR collecting tests/test_typeguard.py ________________________________________
tests/test_typeguard.py:3: in <module>
    import tiledbsoma
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1147: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:690: in _load_unlocked
    ???
/Users/ryan/.pyenv/versions/tiledb-soma-3.11.8/lib/python3.11/site-packages/typeguard/importhook.py:81: in exec_module
    return super().exec_module(module)
src/tiledbsoma/__init__.py:146: in <module>
    from ._collection import Collection
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1147: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:690: in _load_unlocked
    ???
/Users/ryan/.pyenv/versions/tiledb-soma-3.11.8/lib/python3.11/site-packages/typeguard/importhook.py:81: in exec_module
    return super().exec_module(module)
src/tiledbsoma/_collection.py:67: in <module>
    class CollectionBase(  # type: ignore[misc]  # __eq__ false positive
src/tiledbsoma/_collection.py:245: in CollectionBase
    @_funcs.forwards_kwargs_to(
/Users/ryan/.pyenv/versions/tiledb-soma-3.11.8/lib/python3.11/site-packages/typeguard/__init__.py:1032: in wrapper
    check_argument_types(memo)
/Users/ryan/.pyenv/versions/tiledb-soma-3.11.8/lib/python3.11/site-packages/typeguard/__init__.py:875: in check_argument_types
    raise TypeError(*exc.args) from None
E   TypeError: Subscripted generics cannot be used with class and instance checks
================================================ short test summary info ================================================
ERROR tests/test_typeguard.py - TypeError: Subscripted generics cannot be used with class and instance checks
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=================================================== 1 error in 1.75s ====================================================

The culprit seems to be a @_funcs.forwards_kwargs_to annotation in _collection.py:

@_funcs.forwards_kwargs_to(
DataFrame.create, exclude=("context", "tiledb_timestamp")
)
def add_new_dataframe(
self, key: str, *, uri: Optional[str] = None, **kwargs: Any
) -> DataFrame:

Adding @typeguard_ignore to the forwards_kwargs_to declaration didn't help, nor did adding it to the @_funcs.forwards_kwargs_to invocation above.

Open to discussion re: selectively ignoring/fixing problem cases, more explicitly enabling typeguard for known-compliant modules, etc.

@johnkerl johnkerl changed the title [Bug] Typeguard not running as intended [python] Typeguard not running as intended Mar 7, 2024
@ryan-williams
Copy link
Member Author

ryan-williams commented Mar 11, 2024

Thanks to #1116 (comment), my main mistake was thinking typeguard ran as part of a pytest invocation; it is actually run via pre-commit (source).

This (correctly) fails on my repro branch (#2239):

pre-commit run -v --files src/tiledbsoma/_sparse_nd_array.py

pre-commit not running pre-merge

However, it seems pre-commit doesn't run as part of pre-merge checks (python-ci-minimal.yml), which allowed my broken #2239 to pass CI.

"Lint" checks only run for Python 3.10:

run_lint: ${{ matrix.python-version == '3.10' }}

but the build matrix only includes 3.8 and 3.11:

python-version: ['3.8', '3.11']

Looks like this was inadvertent, when upgrading to 3.12 in February: 798ad5c.

I'll work on a PR restoring that, cc @johnkerl. (Update: #2247)

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 a pull request may close this issue.

1 participant