Skip to content

bpo-39573: Add Py_IS_TYPE macro - #18488

Merged
vstinner merged 4 commits into
python:masterfrom
corona10:bpo-39573
Feb 13, 2020
Merged

bpo-39573: Add Py_IS_TYPE macro#18488
vstinner merged 4 commits into
python:masterfrom
corona10:bpo-39573

Conversation

@corona10

@corona10corona10 commented Feb 12, 2020

Copy link
Copy Markdown
Member

@vstinnervstinner 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.

I suggest to modify Argument Clinic in a separated PR, but use this PR to update PyXXX_CheckExact() macros. See: nascheme@c156300

You should credit Neil if you reuse his commit: add Author: Neil Schemenauer <nas-github@arctrix.com> at the end of your commit message.

Comment threadInclude/object.h Outdated
Comment threadDoc/c-api/structures.rst Outdated
@codecov

codecovBot commented Feb 12, 2020

Copy link
Copy Markdown

Codecov Report

Merging #18488 into master will increase coverage by 1.08%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #18488 +/- ##
===========================================
+ Coverage 82.11% 83.19% +1.08% 
===========================================
Files 1956 1571 -385 Lines 589145 414668 -174477 Branches 44447 44450 +3 ===========================================
- Hits 483801 345002 -138799 + Misses 95697 60016 -35681 - Partials 9647 9650 +3 
Impacted FilesCoverage Δ
Lib/distutils/tests/test_bdist_rpm.py30.00% <0.00%> (-65.00%)⬇️
Lib/distutils/command/bdist_rpm.py7.63% <0.00%> (-56.88%)⬇️
Lib/test/test_urllib2net.py76.92% <0.00%> (-13.85%)⬇️
Lib/test/test_smtpnet.py78.57% <0.00%> (-7.15%)⬇️
Lib/ftplib.py63.85% <0.00%> (-6.06%)⬇️
Lib/test/test_ftplib.py87.11% <0.00%> (-4.72%)⬇️
Tools/scripts/db2pickle.py17.82% <0.00%> (-3.97%)⬇️
Tools/scripts/pickle2db.py16.98% <0.00%> (-3.78%)⬇️
Lib/test/test_socket.py71.94% <0.00%> (-3.77%)⬇️
Lib/test/test_asyncio/test_base_events.py91.84% <0.00%> (-3.30%)⬇️
... and 439 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 4fac7ed...e49cc89. Read the comment docs.

@corona10corona10 changed the title bpo-39573: Add Py_IS_TYPE macro[WIP] bpo-39573: Add Py_IS_TYPE macroFeb 12, 2020
Co-Author: Neil Schemenauer <nas-github@arctrix.com>
@corona10corona10 changed the title [WIP] bpo-39573: Add Py_IS_TYPE macrobpo-39573: Add Py_IS_TYPE macroFeb 12, 2020

@corona10corona10 left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@vstinner I've updated the PR :)

Comment threadDoc/c-api/structures.rst Outdated
Comment threadDoc/c-api/structures.rst Outdated
Comment threadMisc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Outdated
@corona10

Copy link
Copy Markdown
MemberAuthor

@vstinner Updated! Thanks for the review

Comment threadDoc/c-api/structures.rst Outdated
Comment threadDoc/c-api/structures.rst Outdated
Comment threadMisc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Outdated
Comment threadMisc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Outdated
Equivalent to: Py_TYPE(o) == type.

.. versionadded:: 3.9

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.

Ooops, I noticed a typo in one of my previous commit. Would you mind to take this PR as an opportunity to fix it?

https://docs.python.org/dev/c-api/structures.html#c.Py_SET_SIZE

"Set the object o size of size." => "Set ... to size."

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Oh sure :)

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.

Wait, @brandtbucher wrote PR #18496. I will ask him to fix the typo there. You can ignore this special request ;-)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Got it ;-)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Current

스크린샷 2020-02-13 오후 11 23 37

Suggestion

스크린샷 2020-02-13 오후 11 15 15

Looks like not work as we want ;)

@corona10

Copy link
Copy Markdown
MemberAuthor

@vstinner Updated :)

@vstinner
vstinner merged commit d905df7 into python:masterFeb 13, 2020
@vstinner

Copy link
Copy Markdown
Member

Thanks, merged. You can now redo your Argument Clinic change on top of it.

Sign up for freeto 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

@corona10@vstinner@the-knights-who-say-ni@bedevere-bot