Skip to content

gh-132054: Add application/yaml to mimetypes#132056

Merged
hugovk merged 9 commits into
python:mainfrom
Kristinita:KiraMediaTypeApplicationYaml
Apr 21, 2025
Merged

gh-132054: Add application/yaml to mimetypes#132056
hugovk merged 9 commits into
python:mainfrom
Kristinita:KiraMediaTypeApplicationYaml

Conversation

@Kristinita

@Kristinita Kristinita commented Apr 3, 2025

Copy link
Copy Markdown
Contributor

@Kristinita Kristinita requested a review from a team as a code owner April 3, 2025 17:43
Comment thread Misc/NEWS.d/next/Library/2025-04-03-20-28-54.gh-issue-132054.c1nlOx.rst Outdated
@AA-Turner AA-Turner changed the title gh-132054 Add application/yaml to mimetypes gh-132054: Add application/yaml to mimetypes Apr 3, 2025
@Kristinita Kristinita requested review from a team, AlexWaygood and JelleZijlstra as code owners April 3, 2025 18:40
@brianschubert

Copy link
Copy Markdown
Contributor

@Kristinita I think you accidentally included some unrelated changes in your last push

@Kristinita

Kristinita commented Apr 3, 2025

Copy link
Copy Markdown
Contributor Author

Type: Reply 💬

@brianschubert, I’m sorry, I made a mistake in the test file and create the new commit that fix tests. I can’t push my changes to GitHub, so I run git fetch, git rebase and then push changes to GitHub.

I can close this pull request, update my CPython fork and send a new pull request. Or should I do something else? I can’t find instructions for this case in Python Developer’s Guide.

I apologize for the concern.

Thanks.

@brianschubert

brianschubert commented Apr 3, 2025

Copy link
Copy Markdown
Contributor

No worries! And no need to open a new PR. You can undo the extra changes by running git restore -s HEAD~1 ./ (which will reset your working tree to what it was before the commit), then remake the intended change to test_mimetypes.py and create a new commit.

@AA-Turner

Copy link
Copy Markdown
Member

@Kristinita please read our AI policy.

A

…mand

python#132056 (comment)
Signed-off-by: Kristinita <Kristinita@users.noreply.github.com>
@Kristinita

Kristinita commented Apr 3, 2025

Copy link
Copy Markdown
Contributor Author

Type: Fixed ✔️

@brianschubert , done. All checks have passed now.

Thanks.

@hugovk

hugovk commented Apr 3, 2025

Copy link
Copy Markdown
Member

Please could you add it to What's New?

https://docs.python.org/3.14/whatsnew/3.14.html#mimetypes

And add a test for .yml?

@Kristinita

Copy link
Copy Markdown
Contributor Author

Type: Question

1. Question

And add a test for .yml?

@hugovk, what is the preferred format for tests for non-preferred extensions like .yml? I was looking for the answer to this in previous pull requests that add new media types, but I can’t find a conventional method.

2. Missing tests

Currently, the file test_mimetypes.py in many cases contains tests solely for preferred extensions. Examples:

  1. For audio/mpeg media type: test_mimetypes.py has the test for the .mp3 preferred extension, but not for the non-preferred extension .mp2.
  2. For video/mpeg media type: test_mimetypes.py has the test for the .mpeg preferred extension, but not for non-preferred extensions .m1v, .mpa, .mpe and .mpg.

3. Existing test for non-preferred extensions

Should I create a test like the test for the text/plain media type?

all = self.db.guess_all_extensions('text/plain', strict=True)
self.assertTrue(set(all) >= {'.bat', '.c', '.h', '.ksh', '.pl', '.txt'})
self.assertEqual(len(set(all)), len(all)) # no duplicates

4. Simpler adding tests

Possibly, it would be nice if adding tests for non-preferred extension will be simpler. For example, the file test_mimetypes.py could have a function like test_non_preferred_extensions to which users will be able to add all non-preferred extensions for media types. Example:

("application/yaml", ".yml"),
("audio/mpeg", ".mp2"),
("video/mpeg", ".m1v"),
("video/mpeg", ".mpa"),
("video/mpeg", ".mpe"),
("video/mpeg", ".mpg")

Thanks.

@hugovk

hugovk commented Apr 4, 2025

Copy link
Copy Markdown
Member

We could add some a test case to check the output of guess_file_type is as expected, for example:

    def test_guess_file_type(self):
        def check_file_type():
            for mime_type, ext in (
                ("application/yaml", ".yaml"),
                ("application/yaml", ".yml"),
            ):
                with self.subTest(mime_type=mime_type, ext=ext):
                    result, _ = mimetypes.guess_file_type(f"filename{ext}")
                    self.assertEqual(result, mime_type)

        check_file_type()
        mimetypes.init()
        check_file_type()

No need to exhaustively fill lots of other preferred+optional extensions, but we could include a few more, such as the audio/mpeg and video/mpeg ones mentioned.

@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@python python deleted a comment Apr 7, 2025

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Updated to add tests and a What's New entry.

The What's New for mimetypes is getting a bit sprawling, but we can copyedit that later as part of #123299.

@Kristinita Thank you and congratulations on your first contribution to CPython!

Comment thread Doc/whatsnew/3.14.rst Outdated
(Contributed by Hugo van Kemenade in :gh:`129965`.)

* Add :rfc:`9512` ``application/yaml`` MIME type for YAML files (``.yaml``
and ``.yml``). (Contributed by Kristinita in :gh:`132056`.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Kristinita We can put your full name here if you like.

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.

@hugovk, thanks for adding the function test_guess_file_type()!

My contribution here is insignificant, and I think it would be more fair if you will add your name instead of my.

(But if the name of the author of the pull request is required, perhaps, Sasha “Nelie” Chernykh instead of Kristinita would be better).

Thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You contribution is not insignificant. I've put both our names there, thanks again!

@hugovk hugovk merged commit 132b6bc into python:main Apr 21, 2025
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