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

fix: new style type annotations don't work for containers and maps #122

Merged
merged 5 commits into from
Sep 27, 2021

Conversation

judahrand
Copy link
Contributor

class MyModel(AvroModel):
    my_dict: dict[str, Any]
    my_list: list[int]
    my_tuple: tuple[str]

Would previously fail

@codecov
Copy link

codecov bot commented Sep 24, 2021

Codecov Report

Merging #122 (52e0959) into master (00841d5) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #122   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files           9        9           
  Lines         788      788           
  Branches      121      121           
=======================================
  Hits          785      785           
  Misses          2        2           
  Partials        1        1           
Impacted Files Coverage Δ
dataclasses_avroschema/fields.py 99.55% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00841d5...52e0959. Read the comment docs.

@judahrand
Copy link
Contributor Author

judahrand commented Sep 24, 2021

@marcosschroh That codecov failure just seems wrong?

@marcosschroh
Copy link
Owner

marcosschroh commented Sep 27, 2021

Looking good. Shall we add a test for python 3.10?

@pytest.mark.skipif(sys.version_info < (3, 10), reason="requires python3.10 or higher")
def test_function():
    # Test containers and maps for 3.10 using dict, list and tuple as types
    ...

@judahrand
Copy link
Contributor Author

judahrand commented Sep 27, 2021

Looking good. Shall we add a test for python 3.10?

@pytest.mark.skipif(sys.version_info < (3, 10), reason="requires python3.10 or higher")
def test_function():
    # Test containers and maps for 3.10 using dict, list and tuple as types
    ...

I'm not sure I understand what you mean?

I added list, tuple and dict to the tests and marked them as expected to fail (which I feel is generally better practice than skipping them but I don't feel that strongly).

Furthermore, this issue isn't a 3.10 issue it's a >=3.9 issue. Hence I mark xfail if below 3.9.

See my changes in consts.py.

@marcosschroh
Copy link
Owner

Oh, sorry. I missed that you add the types 😔 .

SEQUENCE_TYPES = (typing.List, typing.Tuple, typing.Sequence, typing.MutableSequence, list, tuple)   # added list and tuple
MAPPING_TYPES = (typing.Dict, typing.Mapping, typing.MutableMapping, dict)  # added dict

@marcosschroh marcosschroh merged commit 1acfb16 into marcosschroh:master Sep 27, 2021
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.

None yet

2 participants