Permit superclass to subclass lazy typing

tclose commented 10 months ago

Types of changes

Enhancement

Summary

Relaxes typing of lazy fields to allow super-classes to be passed to sub-class fields. Implements #682

Checklist

[x] I have added tests to cover my changes (if necessary)

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.52%. Comparing base (1720ba6) to head (27e7fb8).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #696 +/- ## ========================================== + Coverage 83.35% 83.52% +0.17% ========================================== Files 24 24 Lines 4949 4970 +21 Branches 1411 1415 +4 ========================================== + Hits 4125 4151 +26 + Misses 816 812 -4 + Partials 8 7 -1 ``` | [Flag](https://app.codecov.io/gh/nipype/pydra/pull/696/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/nipype/pydra/pull/696/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype) | `83.52% <100.00%> (+0.17%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tclose commented 9 months ago

NB: rebased on master to pick up new slurm test

djarecka commented 9 months ago

i've restarted the failing slurm test, perhaps we should extend some time in GA

effigies commented 4 months ago

That last commit seems to have broken things.

=================================== FAILURES ===================================
___________________________ test_type_check_nested7 ____________________________
[gw0] linux -- Python 3.12.2 /opt/hostedtoolcache/Python/3.12.2/x64/bin/python

    def test_type_check_nested7():
>       with pytest.raises(TypeError, match="Wrong number of type arguments"):
E       Failed: DID NOT RAISE <class 'TypeError'>

/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/pydra/utils/tests/test_typing.py:159: Failed
_______________________ test_matches_type_tuple_ellipsis _______________________
[gw0] linux -- Python 3.12.2 /opt/hostedtoolcache/Python/3.12.2/x64/bin/python

    def test_matches_type_tuple_ellipsis():
        assert TypeParser.matches_type(ty.Tuple[int], ty.Tuple[int, ...])
        assert TypeParser.matches_type(ty.Tuple[int, int], ty.Tuple[int, ...])
        assert not TypeParser.matches_type(ty.Tuple[int, float], ty.Tuple[int, ...])
>       assert not TypeParser.matches_type(ty.Tuple[int, ...], ty.Tuple[int])
E       AssertionError: assert not True
E        +  where True = <bound method TypeParser.matches_type of <class 'pydra.utils.typing.TypeParser'>>(typing.Tuple[int, ...], typing.Tuple[int])
E        +    where <bound method TypeParser.matches_type of <class 'pydra.utils.typing.TypeParser'>> = TypeParser.matches_type

/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/pydra/utils/tests/test_typing.py:513: AssertionError

tclose commented 4 months ago

That last commit seems to have broken things.

=================================== FAILURES ===================================
___________________________ test_type_check_nested7 ____________________________
[gw0] linux -- Python 3.12.2 /opt/hostedtoolcache/Python/3.12.2/x64/bin/python

    def test_type_check_nested7():
>       with pytest.raises(TypeError, match="Wrong number of type arguments"):
E       Failed: DID NOT RAISE <class 'TypeError'>

/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/pydra/utils/tests/test_typing.py:159: Failed
_______________________ test_matches_type_tuple_ellipsis _______________________
[gw0] linux -- Python 3.12.2 /opt/hostedtoolcache/Python/3.12.2/x64/bin/python

    def test_matches_type_tuple_ellipsis():
        assert TypeParser.matches_type(ty.Tuple[int], ty.Tuple[int, ...])
        assert TypeParser.matches_type(ty.Tuple[int, int], ty.Tuple[int, ...])
        assert not TypeParser.matches_type(ty.Tuple[int, float], ty.Tuple[int, ...])
>       assert not TypeParser.matches_type(ty.Tuple[int, ...], ty.Tuple[int])
E       AssertionError: assert not True
E        +  where True = <bound method TypeParser.matches_type of <class 'pydra.utils.typing.TypeParser'>>(typing.Tuple[int, ...], typing.Tuple[int])
E        +    where <bound method TypeParser.matches_type of <class 'pydra.utils.typing.TypeParser'>> = TypeParser.matches_type

/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/pydra/utils/tests/test_typing.py:513: AssertionError

This is a bit tricky as I can't quite remember what my desired logic was. However, I have decided the tests failed because they were too strict instead of a bug in the code. Basicially, it comes down to whether

tuple[int, ...] should match a target type of tuple[int] (in test_matches_type_tuple_ellipsis4) and likewise list[int] be able to be coerced into tuple[float, float, float]

Given the philosophy of permissive typing at construction time, i.e. if it could be ok, don't raise an error just because it isn't guaranteed to be (error will be raised at runtime if it is a problem), I think both these cases are ok and therefore I have relaxed the tests accordingly

nipype / pydra