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

Email example from python docs doesn't validate: Address as EmailMessage header #2863

Closed
arjennienhuis opened this issue Mar 13, 2019 · 10 comments · Fixed by #2924
Closed

Email example from python docs doesn't validate: Address as EmailMessage header #2863

arjennienhuis opened this issue Mar 13, 2019 · 10 comments · Fixed by #2924
Labels
stubs: false positive Type checkers report false errors

Comments

@arjennienhuis
Copy link

EmailMessage.__setitem__ should support Address as a value. At least the Python docs email example think it does. See https://docs.python.org/3/library/email.examples.html

test_mypy.py:

from email.message import EmailMessage
from email.headerregistry import Address

msg = EmailMessage()
msg['From'] = Address("Pepé Le Pew", "pepe", "example.com")

And then:

$ mypy test_mypy.py
test_mypy.py:5: error: Incompatible types in assignment (expression has type "Address", target has type "Union[str, Header]")
@srittau srittau added stubs: false positive Type checkers report false errors size-small labels Mar 13, 2019
@srittau
Copy link
Collaborator

srittau commented Mar 13, 2019

True. Patches welcome!

@arjennienhuis
Copy link
Author

I'm not sure how. Maybe:

_HeaderType = Union[str, Header, ContentTransferEncodingHeader, ContentTypeHeader, UniqueDateHeader, SingleAddressHeader, AddressHeader, UniqueAddressHeader, UniqueSingleAddressHeader, MIMEVersionHeader, UniqueUnstructuredHeader, DateHeader, ContentDispositionHeader]

Or just:

_HeaderType = Any

@arjennienhuis
Copy link
Author

msg['From'] = 2 actually runs, so maybe:

_HeaderType = object

@gagbo
Copy link

gagbo commented Apr 16, 2019

I don't really like using Any here, it feels like it's defeating the purpose of typing. I prefer the big union _HeaderType as a fix to this problem.

I think BaseHeader (and all the other Header types mentionned above) should be part of the union for _HeaderType, especially since the documentation says that __getitem__ returns a BaseHeader instance more often than not.

I don't know how to make overloads so that if email.headerregistry.HeaderRegistry.use_default_map is True we can type the default mappings correctly.

Also, it would be nice if we were able to deduce the policy from a message, in a way that, for example, message['date'] would be inferred to be an Optional[UniqueDateHeader] for most default policies. But it looks like there's a lot of typing involved for this.

@gagbo
Copy link

gagbo commented Apr 16, 2019

msg['From'] = 2 actually runs, so maybe:

_HeaderType = object

This seems to work because the getitem and setitem have different behaviours : while the getitem mostly returns BaseHeader, the setitem seems to dynamically build a class with a parser which takes any object to build a header from. So I think aliasing _HeaderType to object is not great either

@arjennienhuis
Copy link
Author

But there is this:

msg['Date'] = datetime.datetime(2020, 2, 2)

@arjennienhuis
Copy link
Author

So __getitem__ can return a BaseHeader but __setitem__ should accept object or Any

@gagbo
Copy link

gagbo commented Apr 16, 2019

I'm okay with this too :

  • Add BaseHeader to the union of _HeaderType so email.message.EmailMessage__getitem__ can return a BaseHeader without difficulties.
  • Specify that all specific headers in email.headerregistry are derived from BaseHeader so they can also work there (BaseHeader kind of specifies that the specialized Header have the same interface). This will keep the changes in _HeaderType minimal, and allow all kinds of (Date|Address|...)Header to be used as _HeaderType if need be.
  • Let email.message.EmailMessage.__setitem__ accept anything

@utkarsh2102
Copy link
Contributor

utkarsh2102 commented Apr 17, 2019

_HeaderType = Union[str, Header, ContentTransferEncodingHeader, ContentTypeHeader, UniqueDateHeader, SingleAddressHeader, AddressHeader, UniqueAddressHeader, UniqueSingleAddressHeader, MIMEVersionHeader, UniqueUnstructuredHeader, DateHeader, ContentDispositionHeader]

This workaround makes more sense to me, instead of the above one, no?
At the moment, #2924 implements Any, let me know how should I edit the same.

@arjennienhuis
Copy link
Author

Returning a Union is not ok. See python/mypy#1693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants