Skip to content

Output type of warnings.catch_warnings with record=True #3463

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.

Sign up for GitHub

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

Jump to bottom
Closed
mslapek opened this issue Nov 14, 2019 · 0 comments · Fixed by #3464
Closed

Output type of warnings.catch_warnings with record=True #3463

mslapek opened this issue Nov 14, 2019 · 0 comments · Fixed by #3464

Comments

@mslapek
Copy link
Contributor

mslapek commented Nov 14, 2019 •
edited
Loading

Although warnings.catch_warnings with record=True returns
a list, mypy is not aware of that:

File foo.py:

import warnings

with warnings.catch_warnings(record=True) as w:
    print(len(w))

Command mypy foo.py gives:

$ mypy foo.py
foo.py:4: error: Argument 1 to "len" has incompatible type "Optional[List[_Record]]"; expected "Sized"
Found 1 error in 1 file (checked 1 source file)

Using recent mypy from master (0.750+dev.e97377c454a1d5c019e9c56871d5f229db6b47b2).

I've attached PR fixing this #3464.
@mslapek mslapek mentioned this issue Nov 14, 2019
Merged
srittau pushed a commit that referenced this issue Nov 14, 2019
@mslapek @srittau
Add warnings.catch_warning type hints with Literal. (#3464)
34d68ab 
Closes: #3463
patrakov added a commit to patrakov/certbot that referenced this issue Feb 25, 2020
@patrakov
dns_common_lexicon: deprecate the "domain" parameter for add/del_txt_…
e406510 
…record

There is no valid use case for it, and attempts to use it to find
the correct zone resulted only in bugs when _acme-challenge is a
separate zone. However, external plugins still use it, that's why
deprecation and not complete removal.

Implementation notes:

1. Tests cannot use assertWarns() because of Python 2.7. However,
the call to warnings.catch_warnings(record=True) hits a bug in mypy:
python/typeshed#3463

2. CI tests old plugins against new certbot. That's why both new
and deprecated calls to add/del_txt_record() are accepted as correct
in BaseLexiconAuthenticatorTest.
patrakov added a commit to patrakov/certbot that referenced this issue Feb 25, 2020
@patrakov
dns_common_lexicon: deprecate the "domain" parameter for add/del_txt_…
b85e612 
…record

There is no valid use case for it, and attempts to use it to find
the correct zone resulted only in bugs when _acme-challenge is a
separate zone. However, external plugins still use it, that's why
deprecation and not complete removal.

Implementation notes:

1. Tests cannot use `assertWarns()` because of Python 2.7. However,
the call to `warnings.catch_warnings(record=True)` hits a bug in mypy:
python/typeshed#3463
That's why `assert w is not None`.

2. CI tests old plugins against new certbot. That's why both new
and deprecated calls to add/del_txt_record() are accepted as correct
in BaseLexiconAuthenticatorTest.
patrakov added a commit to patrakov/certbot that referenced this issue Feb 25, 2020
@patrakov
dns_common_lexicon: deprecate the "domain" parameter for add/del_txt_…
465f200 
…record

There is no valid use case for it, and attempts to use it to find
the correct zone resulted only in bugs when _acme-challenge is a
separate zone. However, external plugins still use it, that's why
deprecation and not complete removal.

Implementation notes:

1. Tests cannot use `assertWarns()` because of Python 2.7. However,
the call to `warnings.catch_warnings(record=True)` hits a bug in mypy:
python/typeshed#3463
That's why `assert w is not None`.

2. CI tests old plugins against new certbot. That's why both new
and deprecated calls to add/del_txt_record() are accepted as correct
in BaseLexiconAuthenticatorTest.
patrakov added a commit to patrakov/certbot that referenced this issue Feb 25, 2020
@patrakov
dns_common_lexicon: deprecate the "domain" parameter for add/del_txt_…
f91c539 
…record

There is no valid use case for it, and attempts to use it to find
the correct zone resulted only in bugs when _acme-challenge is a
separate zone. However, external plugins still use it, that's why
deprecation and not complete removal.

Implementation notes:

1. Tests cannot use `assertWarns()` because of Python 2.7. However,
the call to `warnings.catch_warnings(record=True)` hits a bug in mypy:
python/typeshed#3463
That's why `assert w is not None`.

2. CI tests old plugins against new certbot. That's why both new
and deprecated calls to add/del_txt_record() are accepted as correct
in BaseLexiconAuthenticatorTest.
patrakov added a commit to patrakov/certbot that referenced this issue Feb 25, 2020
@patrakov
dns_common_lexicon: deprecate the "domain" parameter for add/del_txt_…
ebf0e34 
…record

There is no valid use case for it, and attempts to use it to find
the correct zone resulted only in bugs when _acme-challenge is a
separate zone. However, external plugins still use it, that's why
deprecation and not complete removal.

Implementation notes:

1. Tests cannot use `assertWarns()` because of Python 2.7. However,
the call to `warnings.catch_warnings(record=True)` hits a bug in mypy:
python/typeshed#3463
That's why `assert w is not None`.

2. CI tests old plugins against new certbot. That's why both new
and deprecated calls to add/del_txt_record() are accepted as correct
in BaseLexiconAuthenticatorTest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add warnings.catch_warning type hints with Literal. mslapek/typeshed
1 participant
@mslapek