Description

JosephSBoyle

opened on Mar 8, 2023

Issue body actions

We can remove almost half of this file without losing any expressiveness and marginally improve performance at the same time.

Linked PRs

• gh-102542 Refactor the mime audio module #102540
• gh-102542 Remove unused bytes object and bytes slicing #106433

Activity

1. JosephSBoyle

   added

   type-featureA feature request or enhancementA feature request or enhancement

   on Mar 8, 2023
2. AlexWaygood

   added

   topic-email

   on Mar 8, 2023
3. warsaw commented on Jul 3, 2023

   

   warsaw

   on Jul 3, 2023

   Member

   More actions

   @JosephSBoyle - thanks for your contributions. I'm commenting here rather than on the PR (although I did add some feedback over there). Please also notice that Lib/email/mime/image.py uses a similar "rules" based mechanism to infer the image/* subtype when not provided.

   One reason I went with this in my rewrite was so that we could easily extend the list of subtypes that we could recognize. Just add another @rule and you're done. While rule() isn't explicitly exposed publicly in either module, one could conceivably extend the list of subtypes in 3rd party code by doing something like:

   from email.mime.audio import rule
   
   @rule
   def _my_image_type(h):
       if h[:5] == 'FLUFL':
           return 'flufl'

   I wouldn't want to lose that implicit extensibility unless there's a good reason to. I'm not sure module size or "marginally improved performance" are compelling enough. Can you provide some performance numbers? Even if they were mind-blowingly better, I'm not sure this is performance critical code.
4. JosephSBoyle commented on Jul 4, 2023

   

   JosephSBoyle

   on Jul 4, 2023

   ContributorAuthor

   More actions

   Hi @warsaw, thanks for taking a look at the PR.

   I wouldn't want to lose that implicit extensibility unless there's a good reason to

   Fair enough, I'm sure that someone is registering their own rules so it makes sense to keep that functionality 😄

   ---

   Would you accept a new, smaller PR to remove the unnecessary header slicing and unused fakefile instance?
5. warsaw commented on Jul 4, 2023

   

   warsaw

   on Jul 4, 2023

   Member

   More actions

   Would you accept a new, smaller PR to remove the unnecessary header slicing and unused fakefile instance?

   I would! Yeah, there's no reason any more to slice the data and it makes sense for audio.py and image.py to have a similar implementation (but not the same implementation because they have different _rules).
6. JosephSBoyle commented on Jul 4, 2023

   

   JosephSBoyle

   on Jul 4, 2023

   ContributorAuthor

   More actions

   Awesome, I've created the new PR here: #106433.

   @warsaw
7. warsaw

   added a commit that references this issue on Jul 5, 2023

   gh-102542 Remove unused bytes object and bytes slicing (#106433)

   70e2a42
8. carljm

   added a commit that references this issue on Jul 5, 2023

   Merge branch 'main' into modulelocals

   4c59e51
9. JosephSBoyle commented on Jul 6, 2023

   

   JosephSBoyle

   on Jul 6, 2023

   ContributorAuthor

   More actions

   Closing as completed.
10. JosephSBoyle

    closed this as completedon Jul 6, 2023

Issue actions