Skip to content

bpo-39684: Combine two if/thens #18557

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

bpo-39684: Combine two if/thens #18557

wants to merge 5 commits into from

Conversation

petdance
Copy link
Contributor

@petdance petdance commented Feb 19, 2020 •
edited by bedevere-bot
Loading

These two code if/thens can be combined

if (ready) {
    kind = PyUnicode_KIND(self);
    data = PyUnicode_DATA(self);
}
else {
    wstr = _PyUnicode_WSTR(self);
}

Py_UCS4 ch;
if (ready) {
    ch = PyUnicode_READ(kind, data, 0);
}
else {
    ch = wstr[0];
}

https://bugs.python.org/issue39684

@codecov
Copy link

codecov bot commented Feb 19, 2020 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18557      +/-   ##
==========================================
- Coverage   82.11%   82.07%   -0.05%     
==========================================
  Files        1955     1955              
  Lines      584054   584123      +69     
  Branches    44457    44458       +1     
==========================================
- Hits       479620   479396     -224     
- Misses      94786    95103     +317     
+ Partials     9648     9624      -24
Impacted Files Coverage Δ
Lib/test/test_curses.py 36.60% <0.00%> (-54.79%) ⬇️
Lib/curses/__init__.py 20.51% <0.00%> (-30.77%) ⬇️
Lib/curses/textpad.py 7.53% <0.00%> (-14.39%) ⬇️
Lib/test/support/__init__.py 69.23% <0.00%> (-0.76%) ⬇️
Lib/_weakrefset.py 59.18% <0.00%> (-0.69%) ⬇️
Lib/test/test_builtin.py 93.51% <0.00%> (-0.47%) ⬇️
Lib/test/test_shutil.py 83.30% <0.00%> (-0.44%) ⬇️
Modules/_io/bufferedio.c 90.01% <0.00%> (-0.39%) ⬇️
Lib/random.py 88.12% <0.00%> (-0.28%) ⬇️
Lib/test/test_io.py 94.81% <0.00%> (-0.10%) ⬇️
... and 8 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 a4ba8a3...38f1c40. Read the comment docs.

@aeros aeros added skip news type-feature A feature request or enhancement labels Feb 19, 2020
@ericvsmith
Copy link
Member

This change looks good to me.

@petdance
Copy link
Contributor Author

Unfortunately it fails the macOS tests. I've been able to recreate it, and I'm investigating why it happens.

ananthan-123 and others added 3 commits February 19, 2020 14:27
@ananthan-123 @petdance
bpo-39572: Document ’total’ flag of TypedDict (GH-18554)
52663b6
@vstinner @petdance
Revert "bpo-38691 Added a switch to ignore PYTHONCASEOK when -E or -I…
3b0cdb9 
… flags passed (#18314)" (GH-18553)

This reverts commit d83b660.
@ananthan-123 @blurb-it @petdance
bpo-39479:Add math.lcm() function: Least Common Multiple (#18547)
38f1c40 
* Update math.rst

* Update math.rst

* updated whats new

* Update test_math.py

* Update mathmodule.c

* Update mathmodule.c.h

* Update ACKS

* 📜🤖 Added by blurb_it.

* Update 3.9.rst

* Update 2020-02-18-12-37-16.bpo-39479.j3UcCq.rst

* Update math.rst

* Update 2020-02-18-12-37-16.bpo-39479.j3UcCq.rst

* Update test_math.py

* Update ACKS

* Update mathmodule.c.h

* Update mathmodule.c

* Update mathmodule.c.h

* Update mathmodule.c.h

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
@petdance
Copy link
Contributor Author

petdance commented Feb 19, 2020 •
edited
Loading

The macOS failures were because of https://bugs.python.org/issue38691

They're now passing once I merged the reverted rebased to pick up the changes to the branch.

@gvanrossum
Copy link
Member

Something went wrong with git. You should probably fetch from upstream and rebase.

@petdance
Copy link
Contributor Author

Something went wrong with git. You should probably fetch from upstream and rebase.

That's exactly what I did, and then pushed it.

@petdance
Copy link
Contributor Author

I can go make a new branch+PR if that would be better.

@gvanrossum
Copy link
Member

gvanrossum commented Feb 20, 2020 via email

@petdance petdance closed this Feb 20, 2020
@petdance petdance deleted the bpo-39684 branch February 20, 2020 01:40
@petdance
Copy link
Contributor Author

New branch coming soon.

@petdance petdance mentioned this pull request Feb 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@ilevkivskyi ilevkivskyi Awaiting requested review from ilevkivskyi

Assignees
No one assigned
Labels
awaiting review skip news type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@petdance @ericvsmith @gvanrossum @the-knights-who-say-ni @bedevere-bot @aeros @ananthan-123 @vstinner