bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #1735

sayanchowdhury · 2017-05-22T23:30:08Z

DeprecationWarning is raised for any use of 'name'.

https://bugs.python.org/issue30274

Automerge-Triggered-By: @brettcannon

ambv · 2017-05-22T23:32:32Z

Lib/importlib/_bootstrap_external.py

+    def __init__(self, fullname=None, path=None, *, name=None):
+        if name is not None:
+            _warnings.warn("the 'name' parameter is deprecated; use "
+                           "'fullname' instead", DeprecationWarning)


You also need to use the deprecated name if it's given.

ambv · 2017-05-23T05:35:23Z

Lib/importlib/_bootstrap_external.py

@@ -910,6 +910,7 @@ def __init__(self, fullname=None, path=None, *, name=None):
        if name is not None:
            _warnings.warn("the 'name' parameter is deprecated; use "
                           "'fullname' instead", DeprecationWarning)
+            self.name = name
        self.name = fullname


You need to put self.name = fullname in an else: statement, otherwise you'll always re-assign self.name to fullname anyway.

Oops, terrible mistake it was. Fixed it.

Mariatta · 2017-06-12T06:50:55Z

I believe this was done during Pycon US sprint, so I applied the sprint label.

berkerpeksag · 2017-07-23T23:56:10Z

Lib/importlib/_bootstrap_external.py

+    def __init__(self, fullname=None, path=None, *, name=None):
+        if name is not None:
+            _warnings.warn("the 'name' parameter is deprecated; use "
+                           "'fullname' instead", DeprecationWarning)


You need to pass stacklevel=2 in order to make DeprecationWarning more useful.

Also, please add a test case for DeprecationWarning.

berkerpeksag · 2017-07-23T23:56:10Z

Misc/NEWS

@@ -338,6 +338,8 @@ Extension Modules

 Library
 -------
+- bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. Patch


We don't edit Misc/NEWS manually anymore. Please use blurb to create a NEWS entry: https://github.com/python/core-workflow/tree/master/blurb

Okay, I will looking in blurb and update the PR accordingly

serhiy-storchaka

The side effect of this change is that it makes both arguments optional, with default to None. I have doubts that this is an intentional change.

The only correct calls are:

ExtensionFileLoader(thename, thepath)
ExtensionFileLoader(thename, path=thepath)
ExtensionFileLoader(fullname=thename, path=thepath)
ExtensionFileLoader(name=thename, path=thepath) # deprecated

Passing both fullname and name should be an error.

bedevere-bot · 2017-11-02T20:44:24Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

brettcannon · 2018-07-20T23:16:29Z

@sayanchowdhury do you plan to address the review comments or should I close it?

sayanchowdhury · 2018-07-24T21:57:17Z

@brettcannon I'll try to get this closed as soon as possible.

sayanchowdhury · 2018-10-13T23:05:06Z

@serhiy-storchaka Which error should I be throwing here? ValueError?

serhiy-storchaka · 2018-10-14T04:09:51Z

TypeError, as in the case when you pass positional and keyword argument for the same parameter.

sayanchowdhury · 2018-10-14T13:55:58Z

Though I've written the test for DeprecationWarning I don't know if it's correct. Locally, my tests are failing with this error

======================================================================
FAIL: test_name_deprecation (test.test_importlib.extension.test_loader.Frozen_LoaderTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sachowdh/code/cpython/Lib/test/test_importlib/extension/test_loader.py", line 34, in test_name_deprecation
    other = self.machinery.ExtensionFileLoader(
AssertionError: DeprecationWarning not triggered

----------------------------------------------------------------------

I tried to import ExtensionFileLoader in Python shell and it does not reflect the updated arguments i.e. fullname. How can I fix it?

➜  cpython git:(bpo30274) ✗ ./python                                                     
Python 3.8.0a0 (heads/bpo30274-dirty:8e5ac1790b, Oct 14 2018, 19:03:35) 
[GCC 8.1.1 20180712 (Red Hat 8.1.1-5)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib.machinery import ExtensionFileLoader
>>> help(ExtensionFileLoader)

class ExtensionFileLoader(FileLoader, _LoaderBasics)
 |  ExtensionFileLoader(name, path)

berkerpeksag · 2018-10-16T18:59:04Z

@sayanchowdhury did you run make regen-importlib?

pythonGH-1735) This patch would also raise DeprecationWarning for 'name'. Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

sayanchowdhury · 2018-10-17T10:20:32Z

@berkerpeksag I did not. Thanks. I have updated the PR.

I don't know if the message for TypeError sounds good. Please review the PR.

berkerpeksag

Please add a NEWS entry. See https://devguide.python.org/committing/#what-s-new-and-news-entries for details.

Lib/test/test_importlib/extension/test_loader.py

pythonGH-1735) This patch would also raise DeprecationWarning for 'name'. Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

bedevere-bot · 2020-03-20T22:49:40Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Doc/whatsnew/3.9.rst

Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

sayanchowdhury · 2020-03-21T16:36:31Z

I'm really sorry to send this back to you, @sayanchowdhury, but I just noticed some indentation inconsistencies. (I would have committed them myself and then merged but I don't think you checked the box to let me edit the PR).

Sorry for that one. Probably because it is an old PR.

sayanchowdhury · 2020-03-21T16:36:41Z

I have made the requested changes; please review again.

bedevere-bot · 2020-03-21T16:36:46Z

Thanks for making the requested changes!

@berkerpeksag, @serhiy-storchaka, @brettcannon, @ambv: please review the changes made to this pull request.

serhiy-storchaka · 2020-03-22T09:12:55Z

👍

brettcannon · 2020-03-25T17:31:59Z

@sayanchowdhury changes looks great! Unfortunately the macOS tests failed for some reason and now there's a merge conflict. If you could fix the merge conflict then hopefully the tests will pass and then we can merge this!

miss-islington · 2020-03-25T17:32:15Z

Sorry, I can't merge this PR. Reason: Pull Request is not mergeable.

csabella · 2020-05-23T01:00:35Z

@sayanchowdhury, please resolve the merge conflict as this has been approved and is ready to merge. Thanks!

DimitrisJim · 2021-05-17T10:59:37Z

Ping. I see this has stalled, in what way can we get it started again?

brettcannon · 2021-05-17T22:39:19Z

@DimitrisJim macOS test failure needs to be resolved and the merge conflict needs to be fixed.

github-actions · 2022-02-19T00:08:36Z

This PR is stale because it has been open for 30 days with no activity.

serhiy-storchaka

The problem is that it makes all parameters optional. ExtensionFileLoader() is now valid.

brettcannon · 2022-05-04T20:13:56Z

At this point I think the risk of breakage outweighs the purity of normalizing the argument name. Thanks to @sayanchowdhury for the PR, but I think this has stalled out too long to merge at this point.

the-knights-who-say-ni added the CLA signed label May 22, 2017

sayanchowdhury changed the title ~~bpo30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader.~~ bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. May 22, 2017

ambv requested changes May 22, 2017

View changes

sayanchowdhury force-pushed the bpo30274 branch from 011bc46 to 260057d Compare May 23, 2017

ambv requested changes May 23, 2017

View changes

sayanchowdhury force-pushed the bpo30274 branch from 260057d to f54b116 Compare May 23, 2017

Mariatta added the sprint label Jun 12, 2017

berkerpeksag requested changes Jul 23, 2017

View changes

Mariatta added needs rebase and removed needs rebase labels Oct 9, 2017

sayanchowdhury force-pushed the bpo30274 branch from f54b116 to 9aee00f Compare Oct 29, 2017

sayanchowdhury requested a review from Oct 29, 2017

sayanchowdhury force-pushed the bpo30274 branch from 9aee00f to 340ba58 Compare Nov 2, 2017

serhiy-storchaka requested changes Nov 2, 2017

View changes

bedevere-bot added the awaiting changes label Nov 2, 2017

sayanchowdhury force-pushed the bpo30274 branch from 340ba58 to 8e5ac17 Compare Oct 13, 2018

sayanchowdhury force-pushed the bpo30274 branch from bf2ad7b to 882bc02 Compare Oct 17, 2018

berkerpeksag requested changes Oct 17, 2018

View changes

Lib/test/test_importlib/extension/test_loader.py Outdated Show resolved Hide resolved

Lib/test/test_importlib/extension/test_loader.py Outdated Show resolved Hide resolved

sayanchowdhury force-pushed the bpo30274 branch from 882bc02 to 1d03f9e Compare Oct 17, 2018

bedevere-bot added the awaiting changes label Mar 20, 2020

brettcannon reviewed Mar 20, 2020

View changes

Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved

Fix the indentation inconsistencies and update doc

c7b25cf

Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

sayanchowdhury force-pushed the bpo30274 branch from b6d8d63 to c7b25cf Compare Mar 21, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 21, 2020

bedevere-bot requested a review from brettcannon Mar 21, 2020

serhiy-storchaka approved these changes Mar 22, 2020

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 22, 2020

brettcannon approved these changes Mar 25, 2020

View changes

brettcannon added the 🤖 automerge label Mar 25, 2020

zkneupper mentioned this pull request May 17, 2021

bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #26195

Closed

github-actions bot added the stale label Feb 19, 2022

achhina mentioned this pull request May 4, 2022

GH-74460: Renamed name argument to fullname for importlib.machinery.ExtensionFileLoader #92300

Closed

4 tasks

serhiy-storchaka reviewed May 4, 2022

View changes

brettcannon mentioned this pull request May 4, 2022

Rename 'name' to 'fullname' argument to ExtensionFileLoader #74460

Closed

brettcannon closed this May 4, 2022

python / cpython Public

bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #1735

bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #1735

sayanchowdhury commented May 22, 2017 •

edited by miss-islington

ambv May 22, 2017

ambv May 23, 2017

sayanchowdhury May 23, 2017

Mariatta commented Jun 12, 2017

berkerpeksag Jul 23, 2017

berkerpeksag Jul 23, 2017

berkerpeksag Jul 23, 2017

sayanchowdhury Jul 24, 2017

serhiy-storchaka left a comment •

edited

bedevere-bot commented Nov 2, 2017

brettcannon commented Jul 20, 2018

sayanchowdhury commented Jul 24, 2018

sayanchowdhury commented Oct 13, 2018

serhiy-storchaka commented Oct 14, 2018 •

edited by brettcannon

sayanchowdhury commented Oct 14, 2018

berkerpeksag commented Oct 16, 2018

sayanchowdhury commented Oct 17, 2018

berkerpeksag left a comment

bedevere-bot commented Mar 20, 2020

sayanchowdhury commented Mar 21, 2020

sayanchowdhury commented Mar 21, 2020

bedevere-bot commented Mar 21, 2020

serhiy-storchaka commented Mar 22, 2020

brettcannon commented Mar 25, 2020

miss-islington commented Mar 25, 2020

csabella commented May 23, 2020

DimitrisJim commented May 17, 2021

brettcannon commented May 17, 2021

github-actions bot commented Feb 19, 2022

serhiy-storchaka left a comment

brettcannon commented May 4, 2022

python / cpython Public

bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #1735

bpo-30274: Rename 'name' to 'fullname' argument to ExtensionFileLoader. #1735

Conversation

sayanchowdhury commented May 22, 2017 • edited by miss-islington

ambv May 22, 2017

Choose a reason for hiding this comment

ambv May 23, 2017

Choose a reason for hiding this comment

sayanchowdhury May 23, 2017

Choose a reason for hiding this comment

Mariatta commented Jun 12, 2017

berkerpeksag Jul 23, 2017

Choose a reason for hiding this comment

berkerpeksag Jul 23, 2017

Choose a reason for hiding this comment

berkerpeksag Jul 23, 2017

Choose a reason for hiding this comment

sayanchowdhury Jul 24, 2017

Choose a reason for hiding this comment

serhiy-storchaka left a comment • edited

bedevere-bot commented Nov 2, 2017

brettcannon commented Jul 20, 2018

sayanchowdhury commented Jul 24, 2018

sayanchowdhury commented Oct 13, 2018

serhiy-storchaka commented Oct 14, 2018 • edited by brettcannon

sayanchowdhury commented Oct 14, 2018

berkerpeksag commented Oct 16, 2018

sayanchowdhury commented Oct 17, 2018

berkerpeksag left a comment

bedevere-bot commented Mar 20, 2020

sayanchowdhury commented Mar 21, 2020

sayanchowdhury commented Mar 21, 2020

bedevere-bot commented Mar 21, 2020

serhiy-storchaka commented Mar 22, 2020

brettcannon commented Mar 25, 2020

miss-islington commented Mar 25, 2020

csabella commented May 23, 2020

DimitrisJim commented May 17, 2021

brettcannon commented May 17, 2021

github-actions bot commented Feb 19, 2022

serhiy-storchaka left a comment

brettcannon commented May 4, 2022

sayanchowdhury commented May 22, 2017 •

edited by miss-islington

serhiy-storchaka left a comment •

edited

serhiy-storchaka commented Oct 14, 2018 •

edited by brettcannon