GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-39573: Add Py_IS_TYPE macro by corona10 · Pull Request #18488 · python/cpython

Conversation

@corona10

@corona10 corona10 commented Feb 12, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue39573

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 12, 2020
@bedevere-bot bedevere-bot added the awaiting review label Feb 12, 2020
@corona10
corona10 force-pushed the bpo-39573 branch from dd3c0d4 to 2457803 Compare February 12, 2020 17:05
vstinner
vstinner reviewed Feb 12, 2020
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

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 <[email protected]> at the end of your commit message.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/object.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/structures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@corona10
corona10 force-pushed the bpo-39573 branch from 2457803 to 1b156b0 Compare February 12, 2020 17:35
@corona10
bpo-39573: Add Py_IS_TYPE macro
42f1b0e
@corona10
corona10 force-pushed the bpo-39573 branch from 1b156b0 to 42f1b0e Compare February 12, 2020 17:36
@codecov

codecov Bot commented Feb 12, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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 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 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@corona10 corona10 changed the title bpo-39573: Add Py_IS_TYPE macro [WIP] bpo-39573: Add Py_IS_TYPE macro Feb 12, 2020
@corona10
corona10 requested review from 1st1, abalkin and pganssle as code owners February 12, 2020 18:09
@corona10
bpo-39573: Update PyXXX_CheckExact() APIs
b31905e 
Co-Author: Neil Schemenauer <[email protected]>
@corona10
corona10 force-pushed the bpo-39573 branch from bda8c2e to b31905e Compare February 12, 2020 18:14
@corona10 corona10 changed the title [WIP] bpo-39573: Add Py_IS_TYPE macro bpo-39573: Add Py_IS_TYPE macro Feb 12, 2020
@corona10
corona10 requested a review from vstinner February 12, 2020 18:33
corona10
corona10 commented Feb 12, 2020
View reviewed changes

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member Author

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.

@vstinner I've updated the PR :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vstinner
vstinner reviewed Feb 12, 2020
View reviewed changes
Comment thread Doc/c-api/structures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/structures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@corona10
bpo-39573: Apply code review
6008716
@corona10
corona10 force-pushed the bpo-39573 branch from 574e60c to 6008716 Compare February 13, 2020 00:46
@corona10

corona10 commented Feb 13, 2020

Copy link
Copy Markdown
Member Author

@vstinner Updated! Thanks for the review
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@corona10
corona10 requested a review from vstinner February 13, 2020 01:33
vstinner
vstinner reviewed Feb 13, 2020
View reviewed changes
Comment thread Doc/c-api/structures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/structures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2020-02-13-01-30-22.bpo-39573.uTFj1m.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/structures.rst
Equivalent to: Py_TYPE(o) == type.

.. versionadded:: 3.9

@vstinner vstinner Feb 13, 2020

Copy link
Copy Markdown
Member

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.

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."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@corona10 corona10 Feb 13, 2020

Copy link
Copy Markdown
Member Author

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.

Oh sure :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Feb 13, 2020

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@corona10 corona10 Feb 13, 2020

Copy link
Copy Markdown
Member Author

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.

Got it ;-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@corona10 corona10 Feb 13, 2020

Copy link
Copy Markdown
Member Author

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.

Current

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

Suggestion

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

Looks like not work as we want ;)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@corona10
bpo-39573: Apply code review
e49cc89
@corona10
corona10 requested a review from vstinner February 13, 2020 14:32
@corona10

corona10 commented Feb 13, 2020

Copy link
Copy Markdown
Member Author

@vstinner Updated :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner merged commit d905df7 into python:master Feb 13, 2020
@bedevere-bot bedevere-bot removed the awaiting review label Feb 13, 2020
@vstinner

vstinner commented Feb 13, 2020

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Mar 12, 2024
[C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type #83754
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 Awaiting requested review from 1st1
@abalkin abalkin Awaiting requested review from abalkin
@pganssle pganssle Awaiting requested review from pganssle
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@corona10 @vstinner @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.