Issue40570
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020-05-08 20:02 by tucked, last changed 2022-04-11 14:59 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 20009 | closed | tucked, 2020-05-08 20:37 | |
| PR 20015 | merged | jaraco, 2020-05-08 23:49 | |
| Messages (11) | |||
|---|---|---|---|
| msg368459 - (view) | Author: David Tucker (tucked) | Date: 2020-05-08 20:02 | |
https://github.com/python/cpython/commit/518835f3354d6672e61c9f52348c1e4a2533ea00#diff-47c8e5750258a08a6dd9de3e9c3774acL741-R804 That diff changed len(platform.uname()) to 5 (from 6). I noticed because we have some code that checks for 6 strs (arguably unnecessary), but I can also think of contrived examples that would break (e.g. platform.uname()[-3]). Interestingly, platform.uname()[5] still works. Was this effect intentional? |
|||
| msg368486 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-08 23:34 | |
It was intentional to address issue35967, although it was meant to remain compatible. Is len(uname()) an important behavior to retain? It feels like an implementation detail to me. |
|||
| msg368487 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-08 23:37 | |
If it is important to retain the `len`, it's probably also important to retain the `[-N]` accesses and possibly other behaviors of a length 6 tuple. |
|||
| msg368491 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-09 00:26 | |
Thanks David for the report and the draft PR (which helped me validate my thinking on the matter). In PR 20015, I've included additional tests. I've also re-written the compatibility functions to rely on the main `__iter__` override. Another situation that's likely to be incompatible is pickleability. Is that important? I'm tempted to make deprecated the use of `uname_result` for anything other than attribute access (maybe with the ability to cast to a tuple, i.e. `tuple(res)`). The problem with namedtuples is that although they provide backward-compatibility for legacy behavior which returned tuples, they also bring that legacy as debt. By deprecating "access by index", that would constrain the interface to two basic usages: attribute access and `iter()` of all values. |
|||
| msg368502 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2020-05-09 08:09 | |
Hi Jason,
to achieve better backwards compatibility, it's probably better to use
the approach taken for CodeInfo in the codecs.py module:
class CodecInfo(tuple):
"""Codec details when looking up the codec registry"""
def __new__(cls, encode, decode, streamreader=None, streamwriter=None,
incrementalencoder=None, incrementaldecoder=None, name=None,
*, _is_text_encoding=None):
self = tuple.__new__(cls, (encode, decode, streamreader,
streamwriter))
self.name = name
self.encode = encode
self.decode = decode
self.incrementalencoder = incrementalencoder
self.incrementaldecoder = incrementaldecoder
self.streamwriter = streamwriter
self.streamreader = streamreader
if _is_text_encoding is not None:
self._is_text_encoding = _is_text_encoding
return self
def __repr__(self):
return "<%s.%s object for encoding %s at %#x>" % \
(self.__class__.__module__, self.__class__.__qualname__,
self.name, id(self))
This used to be a 4 entry tuple and was extended to hold additional
fields. To the outside, it still looks like a 4-tuple in all aspects,
but attribute access permits accessing the additional fields.
--
Marc-Andre Lemburg
eGenix.com
Professional Python Services directly from the Experts (#1, May 09 2020)
>>> Python Projects, Coaching and Support ... https://www.egenix.com/
>>> Python Product Development ... https://consulting.egenix.com/
________________________________________________________________________
::: We implement business ideas - efficiently in both time and costs :::
eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
Registered at Amtsgericht Duesseldorf: HRB 46611
https://www.egenix.com/company/contact/
https://www.malemburg.com/
|
|||
| msg368518 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-09 14:08 | |
Thanks Marc-Andre for the suggestion, but I don't think that approach is viable here. The whole point of issue35967 was to defer the execution of the `.processor` behavior until it was requested. The intention is not to extend the tuple, but to shorten it (at least not to require the last element to be provided at construction time). Perhaps I'm missing something here, so feel free to provide a more concrete example of what you have in mind. Just be cautious not to violate the intention (https://github.com/python/cpython/blob/77c614624b6bf2145bef69830d0f499d8b55ec0c/Lib/platform.py#L784-L787). I created issue40578 to track deprecation of uname for positional access. |
|||
| msg368519 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-09 14:12 | |
New changeset 2c3d508c5fabe40dac848fb9ae558069f0576879 by Jason R. Coombs in branch 'master': bpo-40570: Improve compatibility of uname_result with late-bound .platform (#20015) https://github.com/python/cpython/commit/2c3d508c5fabe40dac848fb9ae558069f0576879 |
|||
| msg368520 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-09 14:14 | |
I've gone ahead and merged PR 20015 to fix the issue, but I'm happy to revisit if a better approach is proposed. |
|||
| msg368528 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2020-05-09 16:07 | |
Ok, let me add some more context: When I wrote the uname interface I was aware that calling the API will take some resources. That's why I added the cache. IMO, that was enough as optimization. Now, you added a late binding optimization for the whole uname return tuple to save the effort of going out to the system and figure our the value using separate APIs or even shell access. I think this would have been better implemented in the various uname() consumers (https://github.com/python/cpython/blob/77c614624b6bf2145bef69830d0f499d8b55ec0c/Lib/platform.py#L898 and below), using a variant of the uname() API, say _uname(), which leaves out the processor information for those APIs which don't need it and only provide the late binding in processor() (which could then also fill in the cache value for uname(). The uname() API would then still do the full lookup, but applications could then use the specialized API to query only the information they need. I don't think that deprecating standard tuple access is an option for the uname() return value, since it's documented to be a tuple. -- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Experts (#1, May 09 2020) >>> Python Projects, Coaching and Support ... https://www.egenix.com/ >>> Python Product Development ... https://consulting.egenix.com/ ________________________________________________________________________ ::: We implement business ideas - efficiently in both time and costs ::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 https://www.egenix.com/company/contact/ https://www.malemburg.com/ |
|||
| msg368534 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2020-05-09 19:12 | |
> you added a late binding optimization for the whole uname return tuple to save the effort of ... shell access. It wasn't to save the effort and it wasn't an optimization. There was a fundamental race condition where it became impossible to implement a `uname` command using Python because `platform.system()` (or anything else that delegated to `platform.uname()`) would unconditionally shell out to `uname`, and this would happen early, before a library could intercept the behavior. See issue35967 for more details on the motivation and challenges. > I think this would have been better implemented... This idea is interesting, but I believe it also falls prey to the same race condition if any code calls `platform.uname()` before a Python-based uname implementation has a chance to monkeypatch the behavior. May I suggest that you either revive the conversation in issue35967 or open a new ticket to discuss improving the implementation so that the details aren't lost in this ticket, which was specifically about compatibility issues? > I don't think that deprecating standard tuple access is an option for the uname() return value, since it's documented to be a tuple. I'll address this concern in the ticket I opened about the deprecation. |
|||
| msg368764 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2020-05-13 07:47 | |
Hi Jason, I think I have to review the whole set of changes again to understand what your motivation is/was. For https://bugs.python.org/issue35967 I had already stated that your use case is not special enough to make the platform.py logic more complex. BTW: Please don't open several different tickets for the same problem. It doesn't really help to see what is going on. I'll reopen the issue35967 to continue the discussion there. Thanks. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:30 | admin | set | github: 84750 |
| 2020-05-13 07:47:25 | lemburg | set | messages: + msg368764 |
| 2020-05-09 19:12:26 | jaraco | set | messages: + msg368534 |
| 2020-05-09 16:07:35 | lemburg | set | messages: + msg368528 |
| 2020-05-09 14:14:14 | jaraco | set | status: open -> closed messages: + msg368520 keywords: + 3.9regression, - patch resolution: fixed stage: patch review -> resolved |
| 2020-05-09 14:12:45 | jaraco | set | messages: + msg368519 |
| 2020-05-09 14:08:22 | jaraco | set | messages: + msg368518 |
| 2020-05-09 08:09:43 | lemburg | set | messages: + msg368502 |
| 2020-05-09 00:26:35 | jaraco | set | messages: + msg368491 |
| 2020-05-08 23:49:32 | jaraco | set | pull_requests: + pull_request19326 |
| 2020-05-08 23:37:31 | jaraco | set | messages: + msg368487 |
| 2020-05-08 23:34:28 | jaraco | set | messages: + msg368486 |
| 2020-05-08 21:21:14 | zach.ware | set | nosy:
+ lemburg, jaraco |
| 2020-05-08 20:37:44 | tucked | set | keywords:
+ patch stage: patch review pull_requests: + pull_request19320 |
| 2020-05-08 20:02:20 | tucked | create | |
➜
