classification
Title: copy.copy() does not properly copy os.environment
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: andrei.avk, anton.barkovsky, gregory.p.smith, htgoebel, josh.r, kamilturek, methane, pitrou, r.david.murray, rhettinger, serhiy.storchaka
Priority: normal Keywords: needs review, patch

Created on 2012-07-16 16:08 by htgoebel, last changed 2021-07-03 20:25 by andrei.avk.

Files
File name Uploaded Description Edit
environcopy.patch anton.barkovsky, 2012-07-16 19:13 Add __copy__ method to os.environ review
environcopy_v2.patch anton.barkovsky, 2012-07-16 19:39 Add __copy__ method to os.environ review
Messages (13)
msg165631 - (view) Author: Hartmut Goebel (htgoebel) Date: 2012-07-16 16:08
Wehn copying os.environ usinf copy.copy(), any manipulation on the copied object will change os.environment, too.

$ python
Python 2.7.3 (default, Apr 22 2012, 07:46:58) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import copy, os
>>> os.environ['TITI']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/UserDict.py", line 23, in __getitem__
    raise KeyError(key)
KeyError: 'TITI'
>>> env = copy.copy(os.environ)
>>> env['TITI'] = 'in den Ferien'
>>> os.environ['TITI']
'in den Ferien'
>>> 

Strictly speaking, this is correct, as the os.environment class is meant to manipulate the environment in the background. But user's expectation is different: he thinks, manipulating the copied object is save and does not effect the environment.
msg165637 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-16 16:53
os.environ is not a dictionary, so it isn't all that surprising that a shallow copy doesn't behave like a shallow copy of a dictionary.  deepcopy does what you'd expect, as does os.environ.copy().

Perhaps it is worth improving this by adding a __copy__ method to os.environ.  Someone would need to propose a patch, and it would be an enhancement.
msg165647 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-16 19:13
Here's a patch.
msg165652 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-16 19:39
A new patch with tests.
msg165691 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-17 08:57
I agree with David that copy(os.environ) is rather ambiguous. I think the preferred idiom should be to call dict(os.environ).
As for __copy__, I don't know what it should do: perhaps simply raise an error when copying is attempted?
msg165712 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-17 11:40
Well, I think the fact that os.environ.copy is explicitly supported (and does indeed do dict(os.environ)) is an argument in favor of giving that same meaning to copy(os.environ).  I think that follows the principle of least surprise.
msg286384 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-27 21:19
If add support for the copy protocol, consider also adding support for deepcopying and pickling.
msg286392 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-01-28 01:02
[Antoine]
> I agree with David that copy(os.environ) is rather ambiguous.
> I think the preferred idiom should be to call dict(os.environ).
> As for __copy__, I don't know what it should do: perhaps
> simply raise an error when copying is attempted?

FWIW, I concur with Antoine on every point.  Also, raising an error when copying seems reasonable ("refuse the temptation to guess" at what the user intended).
msg286473 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2017-01-30 07:51
patch looks OK.
But I prefer `__deepcopy__ = __copy__ = copy`.

I don't know how to support pickling. (should unpickled object
reference os.environ, or copied dict?)
I feel it is different issue.
msg286513 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-01-31 03:43
That patch would be fine except that I really agree with Antoine that an error should be raised.
msg286526 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2017-01-31 12:51
I agree.
os.environ is not dict, it's a proxy of "current" environment which
is not copyable.
So I agree to copy.copy() shouldn't copy os.environ implicitly.
msg384993 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2021-01-13 04:52
Would we remove the functionality of os.environ.copy()? It seems very odd for types to have a .copy() method that works, while not supporting copy.copy, especially when there is zero documentation, on the web or the docstring, to even hint at the difference.

I'm strongly in favor of silently doing the right thing and behaving the same way the .copy() method already behaves; if you want a "copy" of os.environ that still modifies the environment, that's just aliasing it (envalias = os.environ), not copying at all. If you're trying to make a shallow copy, not an alias, you're trying to separate it from the parent, which every other dict-like thing does (assuming immutable values), where os.environ is a very weird exception (for copy.copy, but not the .copy() method).

Can someone give an example where you'd want copy.copy to produce a "shallow copy" that acts like an alias, not an actual independent copy?
msg396922 - (view) Author: Andrei Kulakov (andrei.avk) * Date: 2021-07-03 20:25
I think it may be good to deprecate and discourage use of `os.environ.copy()`, and add a new method `os.environ.asdict()`. And possibly have `__copy__` raise an error pointing users to `os.environ.asdict()`.

I'm not sure what to do about pickling.
History
Date User Action Args
2021-07-03 20:25:16andrei.avksetnosy: + andrei.avk
messages: + msg396922
2021-03-13 19:09:46kamiltureksetnosy: + kamilturek
2021-01-13 04:52:26josh.rsetnosy: + josh.r
messages: + msg384993
2021-01-12 18:26:21iritkatrielsetcomponents: + Library (Lib)
versions: + Python 3.10, - Python 3.7
2017-01-31 12:51:47methanesetmessages: + msg286526
2017-01-31 03:43:52rhettingersetmessages: + msg286513
2017-01-30 07:51:50methanesetnosy: + methane
messages: + msg286473
2017-01-28 01:02:28rhettingersetnosy: + rhettinger
messages: + msg286392
2017-01-27 21:19:35serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg286384
2017-01-27 21:06:01gregory.p.smithsetkeywords: + needs review
nosy: + gregory.p.smith
stage: patch review

versions: + Python 3.7, - Python 3.4
2012-07-17 11:40:18r.david.murraysetmessages: + msg165712
2012-07-17 08:57:05pitrousetnosy: + pitrou
messages: + msg165691
2012-07-16 19:39:01anton.barkovskysetfiles: + environcopy_v2.patch

messages: + msg165652
2012-07-16 19:13:50anton.barkovskysetfiles: + environcopy.patch

nosy: + anton.barkovsky
messages: + msg165647

keywords: + patch
2012-07-16 16:53:35r.david.murraysetversions: + Python 3.4, - Python 2.7
nosy: + r.david.murray

messages: + msg165637

type: behavior -> enhancement
2012-07-16 16:08:57htgoebelcreate