Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-5664: 2to3 convert Cookie.Cookie properly#15268

Closed
aldwinaldwin wants to merge 5 commits into
python:mainpython/cpython:mainfrom
aldwinaldwin:2to3simplecookiealdwinaldwin/cpython:2to3simplecookieCopy head branch name to clipboard
Closed

bpo-5664: 2to3 convert Cookie.Cookie properly#15268
aldwinaldwin wants to merge 5 commits into
python:mainpython/cpython:mainfrom
aldwinaldwin:2to3simplecookiealdwinaldwin/cpython:2to3simplecookieCopy head branch name to clipboard

Conversation

@aldwinaldwin

@aldwinaldwin aldwinaldwin commented Aug 14, 2019

Copy link
Copy Markdown
Contributor

Comment thread Doc/library/2to3.rst
@@ -276,6 +276,13 @@ and off individually. They are described here in more detail.
Handles other modules renames in the standard library. It is separate from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Handles other modules renames in the standard library. It is separate from
Handles other module renames in the standard library. It is separate from

Comment thread Doc/library/2to3.rst

Handles other modules renames in the standard library. It is separate from
the :2to3fixer:`imports` and :2to3fixer:`imports2` fixers only because of
technical limitations. :2to3fixer:`renames` needs to run afterwards to

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these technical limitations?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test_imports() would fail, because this fix uses 2 steps. fix_renames does a second change after fix_imports.

Comment thread Doc/library/2to3.rst

.. 2to3fixer:: imports3

Handles other modules renames in the standard library. It is separate from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Handles other modules renames in the standard library. It is separate from
Handles other module renames in the standard library. It is separate from

'htmlentitydefs' : 'html.entities',
'HTMLParser' : 'html.parser',
'Cookie': 'http.cookies',
#'Cookie': 'http.cookies', is handled by fix_imports3 + renames

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove this line.

else:
# Replace usage of the module.
bare_name = results["bare_with_attr"][0]
if isinstance(results["bare_with_attr"],list):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(results["bare_with_attr"],list):
if isinstance(results["bare_with_attr"], list):

self.transform(node, results)
else:
# Replace usage of the module.
bare_name = results["bare_with_attr"][0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do? (when is results["bare_with_attr"] not a list?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there is only 1 key/value in MAPPING like in fix_imports3.



MAPPING = {
'Cookie': 'http.cookies',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you format indentation in this file?

class Test_imports3(FixerTestCase):

def setUp(self):
super(Test_imports3, self).setUp(['imports3', 'renames'])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you only include imports3 in this test class? You can make a separate test class for the combination of imports3 and renames.

@ericsnowcurrently ericsnowcurrently removed their request for review September 10, 2019 16:05
@brettcannon brettcannon removed their request for review September 11, 2019 12:58
@encukou encukou requested review from benjaminp and removed request for encukou February 18, 2020 12:37
@pablogsal

Copy link
Copy Markdown
Member

@aldwinaldwin Are you still interested on the finish this PR? I could finish it for you if you don't have time :)

@aldwinaldwin

Copy link
Copy Markdown
Contributor Author

@pablogsal Sure, feel free to take over.

@iritkatriel

iritkatriel commented Oct 20, 2021

Copy link
Copy Markdown
Member

Closed under bpo-45544.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.