
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2014-03-29 01:26 bybrandon-rhodes, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| attach_not_property.patch | joegod,2014-08-04 06:35 | review | ||
| multipart_is_property.patch | joegod,2014-08-04 11:15 | review | ||
| multipart_is_property_2.patch | serhiy.storchaka,2014-08-04 12:21 | Softer transition | review | |
| is_attachment_as_method.patch | r.david.murray,2014-09-20 21:00 | review | ||
| Messages (11) | |||
|---|---|---|---|
| msg215104 -(view) | Author: Brandon Rhodes (brandon-rhodes)* | Date: 2014-03-29 01:26 | |
I love properties and think they should be everywhere. But consistency is more important, so I suspect that EmailMessage.is_attachment should be demoted to a normal method. Why? Because if it remains a property then I am likely to first write: if msg.is_attachment: ...and then later, when doing another bit of email logic, write: if msg.is_multipart: ...Unfortunately this second piece of code will give me no error and will appear to run just fine, because bool(a_method) always returns True without a problem or warning or error. But the result will not be what I expect: the if statement's true block will always run, regardless of whether the message is multipart.Since EmailMessage is still provisional, and since no one can use is_attachment yet anyway because it is broken for nearly all attachments, mightn't we make these two features consistent before calling it official? | |||
| msg224688 -(view) | Author: Joseph Godbehere (joegod)* | Date: 2014-08-04 06:35 | |
Patch to change message.is_attachment from a property to a normal method. I've updated the doc and all calls to is_attachment. | |||
| msg224689 -(view) | Author: Alyssa Coghlan (ncoghlan)*![]() | Date: 2014-08-04 06:40 | |
This is your call David - I agree consistency is highly desirable, and having a chance to find and fix this kind of discrepancy is a large part of why we introduced provisional APIs. | |||
| msg224693 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-08-04 07:21 | |
Unfortunately this will silently break existing code because msg.is_attachment will be always true. But it is possible to make EmailMessage.is_attachment a property which returns special callable with the __bool__() method which will emit deprecation warning and call the __call__() method. After some intermediate period (one or two releases) it can be replaced by regular method. | |||
| msg224695 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-08-04 07:24 | |
The alternative is to make EmailMessage.is_multipart a property. | |||
| msg224708 -(view) | Author: Joseph Godbehere (joegod)* | Date: 2014-08-04 11:15 | |
Very good point, Serhiy.Here is an alternative patch, which instead changes Message.is_multipart from a method to a property as per your suggestion. This way incorrect usage should fail noisily.This patch is against the relevant docs, tests, is_multipart and calls to is_multipart only. | |||
| msg224711 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-08-04 12:21 | |
Here is a patch with more soft transition. Message.is_multipart() still works but emits deprecation warning. | |||
| msg224712 -(view) | Author: Alyssa Coghlan (ncoghlan)*![]() | Date: 2014-08-04 12:25 | |
Based on the provisional API status, a faster deprecation plan could be todo Serhiy's patch in a 3.4 maintenance release and the hard break in 3.5 | |||
| msg224869 -(view) | Author: R. David Murray (r.david.murray)*![]() | Date: 2014-08-05 18:19 | |
is_multipart is *not* part of the provisional API, though; only is_attachment is.So per my understanding of the provisional rules, we should either make is_attachment a method in both 3.4 maint and 3.5, or make is_multipart emit a deprecation warning in 3.5. I lean toward the former for backward compatibility reasons, with Serhiy's addition of making it emit a warning if not called in 3.4. | |||
| msg227175 -(view) | Author: R. David Murray (r.david.murray)*![]() | Date: 2014-09-20 21:00 | |
Here is a patch. Sorry for leaving it until the last minute...maybe someone can review it, but it is simple enough I'll commit it soon regardless. | |||
| msg227184 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2014-09-20 22:17 | |
New changeseta3df1c24d586 by R David Murray in branch '3.4':#21091: make is_attachment a method.https://hg.python.org/cpython/rev/a3df1c24d586New changesetf7aff40609e7 by R David Murray in branch 'default':Merge:#21091: make is_attachment a method.https://hg.python.org/cpython/rev/f7aff40609e7 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:00 | admin | set | github: 65290 |
| 2014-09-20 22:18:04 | r.david.murray | set | status: open -> closed type: behavior stage: needs patch -> resolved resolution: fixed versions: + Python 3.4 |
| 2014-09-20 22:17:26 | python-dev | set | nosy: +python-dev messages: +msg227184 |
| 2014-09-20 21:00:36 | r.david.murray | set | files: +is_attachment_as_method.patch messages: +msg227175 |
| 2014-08-06 08:56:28 | serhiy.storchaka | set | stage: commit review -> needs patch |
| 2014-08-05 18:19:20 | r.david.murray | set | messages: +msg224869 |
| 2014-08-04 12:25:11 | ncoghlan | set | messages: +msg224712 |
| 2014-08-04 12:21:21 | serhiy.storchaka | set | files: +multipart_is_property_2.patch messages: +msg224711 |
| 2014-08-04 11:15:55 | joegod | set | files: +multipart_is_property.patch messages: +msg224708 |
| 2014-08-04 07:24:48 | serhiy.storchaka | set | messages: +msg224695 |
| 2014-08-04 07:21:46 | serhiy.storchaka | set | nosy: +serhiy.storchaka messages: +msg224693 |
| 2014-08-04 06:40:02 | ncoghlan | set | nosy: +ncoghlan messages: +msg224689 assignee:r.david.murray stage: commit review |
| 2014-08-04 06:35:54 | joegod | set | files: +attach_not_property.patch nosy: +joegod messages: +msg224688 keywords: +patch |
| 2014-04-04 21:23:39 | eric.araujo | set | nosy: +eric.araujo versions: + Python 3.5, - Python 3.4 |
| 2014-03-29 01:26:19 | brandon-rhodes | create | |