gh-151575: Deprecate passing file paths instead of URLs to mimetypes.guess_type() - #151576
gh-151575: Deprecate passing file paths instead of URLs to mimetypes.guess_type()#151576NaveenKumarG-dev wants to merge 16 commits into
Conversation
Documentation build overview
169 files changed ·
|
…ess_type with new guess_file_type method
…com/NaveenKumarG-dev/cpython into gh-mimetypes-guess-type-deprecation
| .. soft-deprecated:: 3.13 | ||
| Passing a file path instead of URL. | ||
| Use :func:`guess_file_type` for this. | ||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
Schedule removal for Python 3.21. Also, please add a note that it was soft-deprecated since Python 3.13.
There was a problem hiding this comment.
All of these "scheduled for removal" notes should be in Doc/deprecations/pending-removal-in-3.21.rst, not inline in all of the various notes. @NaveenKumarG-dev, could you move them over to there? You'll see some examples of other pending items in that doc directory.
@StanFromIreland, is it right to include a note about the soft-deprecated history? I don't see that on other deprecations.
EDIT: I was mistaken here, see the thread below for more accurate details on what should change. Sorry! 😅
There was a problem hiding this comment.
All of these "scheduled for removal" notes should be in Doc/deprecations/pending-removal-in-3.21.rst, not inline in all of the various notes.
I would prefer we have them here as well, that's our usual process. Otherwise, how is a user meant to know whether the function is going away, compared to just a subset of its functionality?
@StanFromIreland, is it right to include a note about the soft-deprecated history? I don't see that on other deprecations.
Let's include it for clarity. (And that's probably because they weren't soft-deprecated before, it's quite rare to do this.)
There was a problem hiding this comment.
Schedule removal for Python 3.21.
| .. deprecated:: 3.16 | |
| .. deprecated-removed:: 3.16 3.21 |
There was a problem hiding this comment.
I would prefer we have them here as well, that's our usual process.
Oh, now that I'm looking more carefully and tracing this back, this is my mistake! Your statement that this is the norm made me recheck things, and I now see that we have a dedicated directive for this, deprecated-removed.
I believe that the correct course of action here is to update all of these to use deprecated-removed:: 3.16 3.21.
The text which is added here, stating the removal plan, is not typical, and I think should be removed.
Does the soft-deprecation need to be a textual note, or should it be simply a set of stacked directives, like so?
.. soft-deprecated:: 3.13
.. deprecated-removed:: 3.16 3.21
text goes hereThere was a problem hiding this comment.
Setting aside my earlier mistake, I think the correct rewrite is to remove the part of the note which details soft-deprecation and removal schedule, and use the directives instead.
So the rewrite should be something like this:
- .. deprecated:: 3.16
+ .. soft-deprecated:: 3.13
+ .. deprecated-removed:: 3.16 3.21
Passing a file path (or path-like object) instead of a URL.
Use :meth:`guess_file_type` instead.
- Soft-deprecated since Python 3.13, scheduled for removal in Python 3.21.There was a problem hiding this comment.
Using the bare directive makes it appear that the function itself is soft-deprecated, I'd prefer we state it instead to avoid such confusion and duplicating the details. We should however use deprecated-removed, and remove the ", scheduled ..." bit as it's rendered by the directive.
There was a problem hiding this comment.
Hugo made a comment below which suggested using soft-deprecated along with deprecated-removed, but maybe he didn't mean stacking them without a body?
I tried a local doc build with this:

As you say, it looks like the whole function is soft deprecated, which is misleading.
I tried putting a body line in for the soft deprecation and I think I like that best:
Do you prefer a textual note inside of the deprecated-removed directive over this?
There was a problem hiding this comment.
I think a textual note is less noise/clutter/duplication, ultimately, once it's removed its previous soft deprecation is not particularly significant.
There was a problem hiding this comment.
Yep, it would be a bit shorter/simpler. I'm convinced!
In terms of getting this unblocked, I think it would be best if the PR author updates to match your feedback.
@NaveenKumarG-dev, could you implement Stan's recommendation here, which is to add a final line in the deprecated-removed notes that simply says "Soft-deprecated since version 3.13 ?
Sorry for giving misleading feedback earlier.
There was a problem hiding this comment.
Please also remove file paths/path-like objects here.
| type='image/jpeg', strict=True), ['.jpg', '.jpe', '.jpeg']) | ||
|
|
||
|
|
||
| class GuessTypeDeprecationTestCase(unittest.TestCase): |
There was a problem hiding this comment.
This is already covered above, I don't see anything that adds new coverage? We also run our test suite with -Werror on buildbots, so we don't miss spurious warnings.
| """Guess the type of a file which is either a URL or a path-like object. | ||
| """Guess the type of a file based on its URL. | ||
|
|
||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
Please remove this note.
| scheme = p.scheme | ||
| url = p.path | ||
| else: | ||
| # Input has no URL scheme — it is a file path, not a URL. |
There was a problem hiding this comment.
| # Input has no URL scheme — it is a file path, not a URL. |
I think it's sufficiently clear from reading the logic, let's not repeat ourselves.
| @@ -0,0 +1 @@ | |||
| Emit DeprecationWarning when a file path is passed to :func:`mimetypes.guess_type`. Use :func:`mimetypes.guess_file_type` instead. | |||
There was a problem hiding this comment.
Reword it like so please: "Deprecate passing X to X. Use X instead."
|
Please do not use the Update Branch button unless necessary (e.g. fixing conflicts, jogging the CI, or very old PRs) as it uses valuable resources and results in spurious notifications. For more information see the devguide. |
| Guess the type of a file based on its URL, given by *url*. | ||
| URL can be a string. |
There was a problem hiding this comment.
This change seems malformed. "URL can be a string." isn't a normal sentence to have in docs.
I suggest:
| Guess the type of a file based on its URL, given by *url*. | |
| URL can be a string. | |
| Guess the type of a file based on its URL, given as a string. |
| .. soft-deprecated:: 3.13 | ||
| Passing a file path instead of URL. | ||
| Use :func:`guess_file_type` for this. | ||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
All of these "scheduled for removal" notes should be in Doc/deprecations/pending-removal-in-3.21.rst, not inline in all of the various notes. @NaveenKumarG-dev, could you move them over to there? You'll see some examples of other pending items in that doc directory.
@StanFromIreland, is it right to include a note about the soft-deprecated history? I don't see that on other deprecations.
EDIT: I was mistaken here, see the thread below for more accurate details on what should change. Sorry! 😅
| import io | ||
| import mimetypes | ||
| import os | ||
|
|
There was a problem hiding this comment.
Revert these unrelated changes.
| # Lazy import to improve module import time | ||
| import os | ||
| import urllib.parse | ||
| import warnings |
There was a problem hiding this comment.
Use lazy import (and you can move the others as well).
| .. soft-deprecated:: 3.13 | ||
| Passing a file path instead of URL. | ||
| Use :func:`guess_file_type` for this. | ||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
Schedule removal for Python 3.21.
| .. deprecated:: 3.16 | |
| .. deprecated-removed:: 3.16 3.21 |
| Similar to the :func:`guess_type` function, using the tables stored as part of | ||
| the object. | ||
|
|
||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
| .. deprecated:: 3.16 | |
| .. deprecated-removed:: 3.16 3.21 |
| .. deprecated:: 3.16 | ||
| Passing a file path (or path-like object) instead of a URL. | ||
| Use :func:`guess_file_type` instead. | ||
| Soft-deprecated since Python 3.13, scheduled for removal in Python 3.21. |
There was a problem hiding this comment.
We can skip this if we keep the .. soft-deprecated:: 3.13 bit and use .. deprecated-removed:: 3.16 3.21:
| Soft-deprecated since Python 3.13, scheduled for removal in Python 3.21. |
| .. deprecated:: 3.16 | ||
| Passing a file path (or path-like object) instead of a URL. | ||
| Use :meth:`guess_file_type` instead. | ||
| Soft-deprecated since Python 3.13, scheduled for removal in Python 3.21. |
There was a problem hiding this comment.
| Soft-deprecated since Python 3.13, scheduled for removal in Python 3.21. |
| scheme = p.scheme | ||
| url = p.path | ||
| else: | ||
| import warnings |
There was a problem hiding this comment.
In a previous review comment, this was flagged to move into a lazy import warnings at the top of the module.
You should do the same for import urllib.parse, above (and therefore removing it from the location where it appears below).
@StanFromIreland, I think the import os should just move to the top of the module, but not become lazy? Does that sound right to you? My impression is that it's pulled in on interpreter start. I suppose other interpreters might not do that?
| .. soft-deprecated:: 3.13 | ||
| Passing a file path instead of URL. | ||
| Use :func:`guess_file_type` for this. | ||
| .. deprecated:: 3.16 |
There was a problem hiding this comment.
Yep, it would be a bit shorter/simpler. I'm convinced!
In terms of getting this unblocked, I think it would be best if the PR author updates to match your feedback.
@NaveenKumarG-dev, could you implement Stan's recommendation here, which is to add a final line in the deprecated-removed notes that simply says "Soft-deprecated since version 3.13 ?
Sorry for giving misleading feedback earlier.
Context
mimetypes.guess_type()is intended for URLs, whilemimetypes.guess_file_type()was introduced in Python 3.13 as the preferred API for file paths.The implementation still accepts file paths without warning, despite the existing
# TODO: Deprecate accepting file paths (in particular path-like objects)comment inLib/mimetypes.py. This PR implements that TODO by deprecating file path inputs toguess_type()and directing users toguess_file_type()instead.Changes Made
Implementation: Modified
mimetypes.MimeTypes.guess_type()to emit aDeprecationWarning(stacklevel=2) when called with a file path instead of a URL.Coverage: The warning triggers for plain strings that do not contain a valid URL scheme, as well as for
bytesandos.PathLikeobjects. Valid URLs such ashttp://,ftp://, anddata:continue to behave as before.Testing: Added
GuessTypeDeprecationTestCasetoLib/test/test_mimetypes.pycovering path and URL inputs to verify warning behavior and prevent regressions.Documentation:
Doc/library/mimetypes.rstto document the runtime deprecation.Doc/whatsnew/3.16.rstandDoc/deprecations/pending-removal-in-future.rst.NEWS: Added the required NEWS entry using
blurb.Rationale
This change aligns the runtime behavior with the documented distinction between URL handling (
guess_type()) and file-path handling (guess_file_type()), while implementing the existing deprecation TODO in the source code.Linked Issue
Fixes #151575
Code of Conduct and CLA
mimetypes.guess_type()#151575