On Sat, Jul 30, 2011 at 11:11:08PM +0300, Ezio Melotti wrote:
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate? FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information. While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object. If we decide upon something, it can be made consistent. So, the question is, when the timeout argument refers to socket._GLOBAL_DEFAULT_TIME, how should we write the docs. 1. def somesocketmethod(arg1,arg2, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, ...) - I don't see anything is wrong with this. 2. def somesocketmethod(arg1,arg2, timeout=None, ...) - And explain that it actually points to a socket default timeout object, which is odd and can lead to user errors. 3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value) - That's how it is currently explained at some places. If this style is okay, we can change the places were it refers to None to be above. Thanks for your review comments, I have address the remaining ones. -- Senthil