| msg217588 - (view) |
Author: Eric V. Smith (eric.smith) *  |
Date: 2014-04-30 09:51 |
| If you meant to supply a patch, it is missing. And in any event, you need to describe the issue. |
|
|
| msg217686 - (view) |
Author: Yinon Ehrlich (Yinon) |
Date: 2014-05-01 11:40 |
| Use the 'abspath' shortcut instead of 'os.path.abspath' See the attached patch (sorry, forgot to attach before) |
|
|
| msg217688 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2014-05-01 11:47 |
| Wouldn't it be better to switch uses of abspath to be os.path.abspath? os.path is used elsewhere in the file, after all. Brett added "from os.path import abspath" in http://hg.python.org/cpython/rev/686e5d38be42 but I think that import should be deleted and os.path.abspath used directly. |
|
|
| msg217769 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2014-05-02 17:59 |
| IMO either change would not improve the code at all. Suggest closing this. |
|
|
| msg217773 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2014-05-02 18:29 |
| I disagree. It took me longer than I'd like to admit to track down the file history and understand it. I'd like to prevent other people from having to try and understand why it works this way. On the other hand, it looks like people have discovered it: https://mail.python.org/pipermail/tutor/2012-August/090891.html so getting rid of it isn't so simple. If we are going to keep it, we should add a test for it (which actually might exist, I haven't checked yet). |
|
|
| msg218702 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2014-05-17 13:46 |
| I'd prefer to get rid of it, otherwise we might get requests to add all the other os.path functions to the shutil namespace, and I don't think having that kind of "more than one way to do it" serves anyone. I suppose we'll have to deprecate it first if we do get rid of it :(. |
|
|
| msg218725 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2014-05-18 00:48 |
| Here is a patch to deprecate the shutil.abspath function. |
|
|
| msg221626 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2014-06-26 17:39 |
| Shouldn't the existing calls to abspath() be changed to os.path.abspath()? Or are both patches meant to be applied? I don't think the first patch applies cleanly any more. In any event: the deprecation and test look good to me. So assuming we get rid of the import and get rid of direct calls to abspath(), I'm +1. |
|
|
| msg221628 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2014-06-26 17:56 |
| Now that I think about it, maybe we don't need a deprecation warning. http://legacy.python.org/dev/peps/pep-0008/#public-and-internal-interfaces says: "Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's __init__ module that exposes functionality from submodules." abspath isn't in __all__, so it's arguably not part of the public API, anyway. |
|
|
| msg225752 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-08-23 16:42 |
| I'm for get rid of "from import" without deprecation. Definitely this is not part of API. |
|
|
| msg227022 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2014-09-18 02:10 |
| New changeset ab369d809200 by Berker Peksag in branch 'default': Issue #21391: Use os.path.abspath in the shutil module. https://hg.python.org/cpython/rev/ab369d809200 |
|
|
| msg227023 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2014-09-18 02:11 |
| Done. Thanks for the reviews! |
|
|