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

Created on 2012-07-16 16:08 by htgoebel, last changed 2017-01-31 12:51 by inada.naoki.

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 (11)
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 (inada.naoki) * (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 (inada.naoki) * (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.
History
Date User Action Args
2017-01-31 12:51:47inada.naokisetmessages: + msg286526
2017-01-31 03:43:52rhettingersetmessages: + msg286513
2017-01-30 07:51:50inada.naokisetnosy: + inada.naoki
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