- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
bpo-31581: Reduce the number of imports for functools #3757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| I added 'skip news' tag, since #1269 don't have news too. | 
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.
The functools.singledispatch change seems reasonable to me, since the cost of checking the import cache twice will be cheap compared to the overhead of defining a new function to apply @singledispatch to.
As per the comments on the argparse PR, this should be accompanied by a comment explaining the trade-off (i.e. there are many programs that use functools without singledispatch, so we trade-off making singledispatch marginally slower for the benefit of making start-up of such applications slightly faster)
However, I'm less sure of the appropriateness of the start-up vs runtime cost trade-off for Counter.most_common, so I'd prefer to see that change in a separate PR.
| A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase  | 
a0082b6    to
    f228a84      
    Compare
  
            
          
                Lib/functools.py
              
                Outdated
          
        
      | # There are many programs that use functools without singledispatch, so we | ||
| # trade-off making singledispatch marginally slower for the benefit of | ||
| # making start-up of such applications slightly faster. | ||
| from types import MappingProxyType | 
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.
It is slightly faster to import types and use types.MappingProxyType once than import MappingProxyType from types. The from ... import case runs a Python code from _bootstrap.py, while the former case runs just a C code if the types module already is imported.
        
          
                Lib/test/test_functools.py
              
                Outdated
          
        
      | def clear(self): | ||
| self.data.clear() | ||
| _orig_wkd = functools.WeakKeyDictionary | ||
| _orig_wkd = weakref.WeakKeyDictionary | 
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.
Use try ... finally or test.support.swap_attr() for guaranteeing that weakref.WeakKeyDictionary is restored if the following checks fail.
| 
 fixed. | 
Since functools and collections are very commonly used, it's worth enough to optimize import.
https://bugs.python.org/issue31581