Skip to content
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

bpo-39815: add cached_property to all #18726

Merged
merged 1 commit into from Mar 1, 2020
Merged

bpo-39815: add cached_property to all #18726

merged 1 commit into from Mar 1, 2020

Conversation

hakancelikdev
Copy link
Contributor

@hakancelikdev hakancelikdev commented Mar 1, 2020

@hakancelikdev hakancelikdev changed the title Bpo 39815 Bpo 39815 - add cached_property to all Mar 1, 2020
@hakancelikdev hakancelikdev changed the title Bpo 39815 - add cached_property to all bpo-39815 - add cached_property to all Mar 1, 2020
@hakancelikdev hakancelikdev changed the title bpo-39815 - add cached_property to all bpo-39815: add cached_property to all Mar 1, 2020
Copy link
Sponsor Member

@isidentical isidentical left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Just a minor comment, otherwise LGTM

Lib/functools.py Outdated
@@ -12,7 +12,7 @@
__all__ = ['update_wrapper', 'wraps', 'WRAPPER_ASSIGNMENTS', 'WRAPPER_UPDATES',
'total_ordering', 'cmp_to_key', 'lru_cache', 'reduce',
'TopologicalSorter', 'CycleError',
'partial', 'partialmethod', 'singledispatch', 'singledispatchmethod']
'partial', 'partialmethod', 'singledispatch', 'singledispatchmethod', "cached_property"]
Copy link
Member

Choose a reason for hiding this comment

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

Could you move this to the next line so the line remains no longer than ~80 chars?

@bedevere-bot
Copy link

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@pablogsal pablogsal added the needs backport to 3.8 only security fixes label Mar 1, 2020
@hakancelikdev
Copy link
Contributor Author

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@pablogsal: please review the changes made to this pull request.

@pablogsal
Copy link
Member

Thanks for the PR, @hakancelik96 !

@codecov
Copy link

codecov bot commented Mar 1, 2020

Codecov Report

Merging #18726 into master will decrease coverage by 1.14%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #18726       +/-   ##
===========================================
- Coverage   83.28%   82.13%    -1.15%     
===========================================
  Files        1571     1955      +384     
  Lines      415244   584723   +169479     
  Branches    44484    44489        +5     
===========================================
+ Hits       345821   480266   +134445     
- Misses      59773    94808    +35035     
+ Partials     9650     9649        -1     
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 435 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 768d739...a6f350f. Read the comment docs.

@hakancelikdev
Copy link
Contributor Author

hakancelikdev commented Mar 1, 2020

Thanks for the PR, @hakancelik96 !

You are welcome.
Thanks for the review @pablogsal and helping of @isidentical. 💯

@miss-islington
Copy link
Contributor

@hakancelik96: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 217dce9 into python:master Mar 1, 2020
@miss-islington
Copy link
Contributor

Thanks @hakancelik96 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry @hakancelik96, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 217dce9ee6e3cf27a0cedbe1e4a6455776373ec2 3.8

@miss-islington miss-islington self-assigned this Mar 1, 2020
sthagen added a commit to sthagen/python-cpython that referenced this pull request Mar 1, 2020
bpo-39815: add cached_property to all (pythonGH-18726)
@bedevere-bot
Copy link

GH-18728 is a backport of this pull request to the 3.8 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label Mar 1, 2020
pablogsal pushed a commit to pablogsal/cpython that referenced this pull request Mar 1, 2020
Automerge-Triggered-By: @pablogsal.
(cherry picked from commit 217dce9)

Co-authored-by: Hakan Çelik <hakancelik96@outlook.com>
pablogsal added a commit that referenced this pull request Mar 2, 2020
Automerge-Triggered-By: @pablogsal.
(cherry picked from commit 217dce9)

Co-authored-by: Hakan Çelik <hakancelik96@outlook.com>
@bedevere-bot

This comment has been minimized.

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE RHEL8 LTO + PGO 3.8 has failed when building commit 12b7143.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/392/builds/33) and take a look at the build logs.
  4. Check if the failure is related to this commit (12b7143) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/392/builds/33

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 5, done.        
remote: Counting objects:  20% (1/5)        
remote: Counting objects:  40% (2/5)        
remote: Counting objects:  60% (3/5)        
remote: Counting objects:  80% (4/5)        
remote: Counting objects: 100% (5/5)        
remote: Counting objects: 100% (5/5), done.        
remote: Total 6 (delta 5), reused 5 (delta 5), pack-reused 1        
From https://github.com/python/cpython
 * branch                  3.8        -> FETCH_HEAD
Reset branch '3.8'

find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make[2]: [Makefile:1778: clean] Error 1 (ignored)

sobolevn added a commit to sobolevn/cpython that referenced this pull request Mar 7, 2020
I have noticed that `'` quotes are used everywhere except this particular case,
which was introduced in python#18726

So, this is a simple fix to enforce better consistency.
miss-islington pushed a commit that referenced this pull request Mar 11, 2020
I have noticed that `'` quotes are used everywhere except this particular case,
which was introduced in #18726

So, this is a trivial fix to enforce better consistency.
@hakancelikdev hakancelikdev deleted the bpo-39815 branch March 30, 2020 13:21
@hakancelikdev hakancelikdev restored the bpo-39815 branch July 6, 2020 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants