GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-27485: Rename and deprecate undocumented functions in urllib.parse by csabella · Pull Request #2205 · python/cpython

Conversation

csabella commented Jun 14, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

The following functions were undocumented:
splitattr, splithost, splitnport, splitpasswd, splitport, splitquery, splittag, splittype, splituser, splitvalue, to_bytes, unwrap

A note in the 2.7 documentation for urllib stated that Python 3 does not expose these helper functions, but yet they were still available.

https://bugs.python.org/issue27485

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

the-knights-who-say-ni added the CLA signed label

Jun 14, 2017

csabella force-pushed the bpo27485 branch from e5f225e to d27233f Compare

June 21, 2017 00:14

csabella force-pushed the bpo27485 branch from d27233f to bab5c0d Compare

July 1, 2017 17:43

vadmium reviewed

Jul 2, 2017

View reviewed changes

Comment thread

Lib/lib2to3/fixes/fix_urllib.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I didn’t see a test case, but won’t this change make 2to3 code call the internal unsupported functions? I think it is better to keep calling the original functions, even if they are deprecated.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes, thank you, this makes more sense.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/urllib/parse.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Does the signature have to change?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

When I was researching the codebase to see what changes needed to be made for a deprecation warning, I found dist and linux_distribution in platform and test_platform, so I copied the tests from there. That change altered the signature from the original to the internal version of the function. I didn't understand why, so I applied a similar change and I probably shouldn't have. I've removed it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/urllib/parse.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Incompatible signature

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This was just careless. Sorry that I made the mistake. I've corrected all the signatures.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_urlparse.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If you tested the proper function, it would reveal that the new signature is incompatible.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I understand this better now. Thank you for reviewing and for your comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/urllib/parse.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This does not seem to be used anywhere else. Why not merge the two functions?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Do you mean have splitport and splitnport call one internal function?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/urllib/parse.py Outdated

vadmium Jul 2, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don’t see how urlparse is a substitute for this

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella Jul 3, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you. You're right, urlparse wasn't a good substitute. I was thinking that the query part of the result from urlparse would be similar to splitvalue, but that's closer to splitquery and still would need extra parsing. I've changed it to use urllib.parse.parse_qsl.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csabella force-pushed the bpo27485 branch from bab5c0d to a62bd79 Compare

July 2, 2017 23:48

csabella force-pushed the bpo27485 branch from 4097a06 to 9330e18 Compare

July 21, 2017 12:49

brettcannon added the awaiting review label

Feb 2, 2018

ambv commented Apr 18, 2018

Copy link

Copy Markdown

Contributor

Looks like this unfortunately didn't make it into 3.7. There's a conflict that would need to be solved and the deprecation messages would need to be renamed to mention "3.8".

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

csabella added 2 commits

April 25, 2018 13:56


          bpo-27485: Deprecate undocumented functions into urllib.parse

ea4d9be


          bpo-27485: Rename and deprecate undocumented functions in urllib.parse

e6f276e

csabella force-pushed the bpo27485 branch from 9330e18 to e6f276e Compare

April 25, 2018 17:57

csabella requested a review from a team as a code owner

April 25, 2018 17:57

csabella added 2 commits

April 25, 2018 14:02


          Update deprecation messages to 3.8

959d531


          Add blurb

csabella commented Apr 25, 2018

Copy link

Copy Markdown

Contributor Author

@ambv Thank you for the review. I've rebased and updated the deprecation messages to reference 3.8 instead of 3.7.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ambv merged commit 0250de4 into python:master

Apr 25, 2018

bedevere-bot removed the awaiting review label

Apr 25, 2018

ambv commented Apr 25, 2018

Copy link

Copy Markdown

Contributor

Thanks! ✨ 🍰 ✨

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

csabella deleted the bpo27485 branch

April 26, 2018 21:58

yan12125 mentioned this pull request

Jul 28, 2018

future.moves.urllib.request broken with Python 3.8 PythonCharmers/python-future#350

Closed

behnam reviewed

Aug 15, 2018

View reviewed changes

Comment thread

Lib/test/test_urlparse.py

		urllib.parse.splittype('')
		self.assertEqual(str(cm.warning),
		'urllib.parse.splittype() is deprecated as of 3.8, '
		'use urllib.parse.urlparse() instead')

behnam Aug 15, 2018

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

From https://docs.python.org/3.8/library/urllib.parse.html#urllib.parse.urlsplit:

This is similar to urlparse(), but does not split the params from the URL. This should generally be used instead of urlparse() if the more recent URL syntax allowing parameters to be applied to each segment of the path portion of the URL (see RFC 2396) is wanted.

Based on this, looks like it's better to recommend using urllib.parse.urlsplit() instead of urllib.parse.urlparse().

What do you think?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

brianschubert mentioned this pull request

Jul 26, 2025

[urllib] Add deprecated functions python/typeshed#14457

Closed

Reviewers

vadmium

vadmium left review comments

+1 more reviewer

behnam

behnam left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

csabella commented Jun 14, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csabella Jul 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ambv commented Apr 18, 2018

Uh oh!

csabella commented Apr 25, 2018

Uh oh!

ambv commented Apr 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

csabella commented Jun 14, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

csabella Jul 3, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.