Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Aug 25, 2024

Changes in the urllib.parse module:

  • Add option allow_none missing_as_none in urlparse(), urlsplit() and urldefrag(). If it is true, represent not defined components as None instead of an empty string.
  • Add option keep_empty in urlunparse() and urlunsplit(). If it is true, keep empty non-None components in the resulting string. By default it is the same as the allow_none value for the result of the urlparse() and urlsplit() calls.
  • Add option keep_empty in the geturl() method of DefragResult, SplitResult, ParseResult and the corresponding bytes counterparts.

…I components

Changes in the urllib.parse module:

* Add option allow_none in urlparse(), urlsplit() and urldefrag(). If
  it is true, represent not defined components as None instead of an
  empty string.
* Add option keep_empty in urlunparse() and urlunsplit(). If it is
  true, keep empty non-None components in the resulting string.
* Add option keep_empty in the geturl() method of DefragResult,
  SplitResult, ParseResult and the corresponding bytes counterparts.
@serhiy-storchaka serhiy-storchaka force-pushed the urllib-parse-allow-none branch from 7032015 to a60c9be Compare August 31, 2024 09:55
@serhiy-storchaka serhiy-storchaka marked this pull request as ready for review November 27, 2024 11:16
@serhiy-storchaka
Copy link
Member Author

It is now ready to review. The status of allow_none is now saved in the DefragResult, SplitResult and ParseResult objects, so in most cases there is no need to pass the keep_empty argument. geturl() no longer needs the keep_empty parameter.

Unfortunately, these objects now have __dict__ and no longer immutable. This is because non-empty __slots__ is not compatible with tuple subclasses. This is a separate complex issue. I'll try to find a solution of it, but it may be difficult.

The long term plan is to make allow_none True by default, and later deprecate allow_none=False. keep_empty=False can still be useful.

@serhiy-storchaka serhiy-storchaka marked this pull request as draft November 28, 2024 09:14
@serhiy-storchaka serhiy-storchaka marked this pull request as ready for review December 5, 2024 11:10
@serhiy-storchaka
Copy link
Member Author

I am sorry, I forget to copy the _keep_empty attribute in copying/encoding/decoding methods. Now the PR is ready for review.

@orsenthil, @barneygale, could you please make a review?

@barneygale barneygale self-requested a review December 5, 2024 14:03
@merwok
Copy link
Member

merwok commented Nov 17, 2025

I think that allow_none is not the correct name: this is not about allowing nones in the input (like how allow_fragments allows fragment in the input), but about controlling the data returned.

Possible alternatives:

  • use_none (short)
  • missing_as_none
  • use_none_for_empty

@serhiy-storchaka
Copy link
Member Author

Maybe missing_as_empty? In future the behavior will be changed to use None for missing by default.

@merwok
Copy link
Member

merwok commented Nov 17, 2025

I think it should be missing_as_empty_string (short for return missing values as empty strings), which is long!

To change the default in the future, do you plan on adding a warning first?
I could see many projects not paying attention to the new param and getting surprised (or silent bugs) a few years after.

@serhiy-storchaka
Copy link
Member Author

To change the default in the future, do you plan on adding a warning first?

On one hand, a warning will inform everyone about the change (it should be a FutureWarning). You will have to pass explicit True or False to silence it and get your behavior. This how we normally do. On other hand, the warning will unnecessary disturb those who are fine with any behavior. We will discuss this when the time come.

@serhiy-storchaka
Copy link
Member Author

I think it should be missing_as_empty_string (short for return missing values as empty strings), which is long!

Currently, missing and empty component are not distinguishable. This parameter will allow to distinguish them. This PR adds also the keeps_empty parameter. So, "empty" refers not only to string, but to component. missing_component_as_empty_component, or simply missing_as_empty. Will it work?

@merwok
Copy link
Member

merwok commented Nov 17, 2025

Ah your’re right, there is empty component and empty string.

missing_as_none is good!

@hugovk hugovk removed their request for review January 21, 2026 21:18
@orsenthil
Copy link
Member

orsenthil commented Jan 22, 2026

Looks like we maintain a patch for the iOS build which is failing to apply with this PR.

  export PATH=/Users/runner/work/cpython/cpython/Apple/iOS/Resources/bin:/Users/runner/work/cpython/cpython/cross-build/arm64-apple-ios-simulator/prefix:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin
  > make -j 3
  arm64-apple-ios-simulator-clang -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -O3 -Wall -mios-version-min=13.0   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Programs/python.o ../../Programs/python.c
  arm64-apple-ios-simulator-clang -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -O3 -Wall -mios-version-min=13.0   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Parser/token.o ../../Parser/token.c
  1 out of 1 hunks failed while patching 'Lib/test/test_urlparse.py'

@orsenthil
Copy link
Member

This file https://github.com/python/cpython/blob/main/Mac/Resources/app-store-compliance.patch (a meta-patch) needs to be updated for the new behavior (returning None instead of '') for it apply cleanly.

@orsenthil
Copy link
Member

orsenthil commented Jan 22, 2026

This file https://github.com/python/cpython/blob/main/Mac/Resources/app-store-compliance.patch should be updated with this content.

diff --git a/Mac/Resources/app-store-compliance.patch b/Mac/Resources/app-store-compliance.patch
index f4b7decc01c..e5d1e7da866 100644
--- a/Mac/Resources/app-store-compliance.patch
+++ b/Mac/Resources/app-store-compliance.patch
@@ -1,21 +1,21 @@
 diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py
-index d6c83a75c1c..19ed4e01091 100644
+index 49a292df934..e1669a0c9b2 100644
 --- a/Lib/test/test_urlparse.py
 +++ b/Lib/test/test_urlparse.py
-@@ -237,11 +237,6 @@ def test_roundtrips(self):
-               '','',''),
+@@ -282,11 +282,6 @@ def test_qs(self, orig, expect):
+               None,None,None),
               ('git+ssh', '[email protected]','/user/project.git',
-               '', '')),
+               None, None)),
 -            ('itms-services://?action=download-manifest&url=https://example.com/app',
--             ('itms-services', '', '', '',
--              'action=download-manifest&url=https://example.com/app', ''),
+-             ('itms-services', '', '', None,
+-              'action=download-manifest&url=https://example.com/app', None),
 -             ('itms-services', '', '',
--              'action=download-manifest&url=https://example.com/app', '')),
+-              'action=download-manifest&url=https://example.com/app', None)),
              ('+scheme:path/to/file',
-              ('', '', '+scheme:path/to/file', '', '', ''),
-              ('', '', '+scheme:path/to/file', '', '')),
+              (None, None, '+scheme:path/to/file', None, None, None),
+              (None, None, '+scheme:path/to/file', None, None)),
 diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py
-index 8f724f907d4..148caf742c9 100644
+index e917f8b61bb..8575172573f 100644
 --- a/Lib/urllib/parse.py
 +++ b/Lib/urllib/parse.py
 @@ -59,7 +59,7 @@

And patch application is successful.

$patch -p1 < Mac/Resources/app-store-compliance.patch
patching file 'Lib/test/test_urlparse.py'
patching file 'Lib/urllib/parse.py'

And

 ./python.exe Lib/test/test_urlparse.py -v

was successful with this change.

@serhiy-storchaka serhiy-storchaka requested a review from a team as a code owner January 22, 2026 11:04
@serhiy-storchaka
Copy link
Member Author

Thank you @orsenthil. I was confused by this failure. The original issue is interesting: #120522.

@serhiy-storchaka serhiy-storchaka merged commit c5cfcdf into python:main Jan 22, 2026
47 checks passed
@serhiy-storchaka serhiy-storchaka deleted the urllib-parse-allow-none branch January 22, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants