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 — gh-89727: Fix os.walk RecursionError on deep trees by jonburdo · Pull Request #99803 · python/cpython

Conversation

@jonburdo

@jonburdo jonburdo commented Nov 27, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Use a stack to implement os.walk iteratively instead of recursively to avoid hitting recursion limits on deeply nested trees.

  • Issue: Change shutil.rmtree and os.walk to support very deep hierarchies #89727

Sorry, something went wrong.

Uh oh!

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

wimglenn, 0x2b3bfa0, casperdcl, and efiop reacted with thumbs up emoji 0x2b3bfa0, skshetry, casperdcl, and efiop reacted with rocket emoji
All reactions
  • 👍 4 reactions
  • 🚀 4 reactions
@bedevere-bot bedevere-bot mentioned this pull request Nov 27, 2022
Change shutil.rmtree and os.walk to support very deep hierarchies #89727
Open
@bedevere-bot bedevere-bot added the awaiting review label Nov 27, 2022
@bedevere-bot

bedevere-bot commented Nov 27, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

All reactions

Sorry, something went wrong.

Uh oh!

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

@ghost

ghost commented Nov 27, 2022 •
edited by ghost
Loading

Uh oh!

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

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

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

@careljonkhout

careljonkhout commented Nov 29, 2022

Copy link
Copy Markdown

These changes look good to me. I would sure like to see this issue fixed. Please add a news entry as suggested by the belverdere bot, so that this PR can move along.

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@jonburdo
jonburdo force-pushed the iterative-os-walk branch from eeca07b to 694c71e Compare November 30, 2022 01:49
@jonburdo
pythongh-89727: Fix os.walk RecursionError on deep trees
99f4691
Use a stack to implement os.walk iteratively instead of recursively to
avoid hitting recursion limits on deeply nested trees.
@jonburdo
jonburdo force-pushed the iterative-os-walk branch from 694c71e to 99f4691 Compare December 15, 2022 16:03
@carljm

carljm commented Dec 15, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Just a note, it's better to push additional commits instead of force-pushing an amended commit. The latter invalidates previous reviews and generally makes it harder for reviewers to see the changes over time. The multiple commits will get squashed on merge anyway. (This also applies to updating from main: merge in main, don't rebase.)

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

carljm
carljm requested changes Dec 15, 2022
View reviewed changes

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

Code changes LGTM.

I think a test should be added here that the recursion limit does not limit depth for os.walk, to prevent regression of the bug. A super-deep directory structure should not be needed for this test, because the test can temporarily set a low recursion limit. It looks like test.support.infinite_recursion can be used for this, although it's badly named for the purpose; should probably move the body of it to a new test.support.temp_recursion_limit function (with no default depth value) and make test.support.infinite_recursion a thin wrapper around it that just provides the default value of 75.

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Dec 15, 2022
@carljm

carljm commented Dec 15, 2022

Copy link
Copy Markdown
Member

(It's not clear to me from the logs whether the Win x64 test failure is related or just flaky; if it fails again after update, then it deserves further investigation.)

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented Dec 15, 2022

Copy link
Copy Markdown
Member

The Windows error (PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'D:\\a\\cpython\\cpython\\build\\test_python_6880�\\test_python_worker_1068�') looks like a flake to me; I've seen it randomly show up before.

carljm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

zmievsa
zmievsa suggested changes Dec 15, 2022
View reviewed changes
Comment thread Lib/os.py Outdated
@@ -343,86 +343,96 @@ def walk(top, topdown=True, onerror=None, followlinks=False):
return _walk(fspath(top), topdown, onerror, followlinks)

def _walk(top, topdown, onerror, followlinks):

@zmievsa zmievsa Dec 15, 2022

Copy link
Copy Markdown
Contributor

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 we remove the _walk function and simply put everything under walk because the second one was only necessary because of sys.audit not making sense for recursive calls.

Sorry, something went wrong.

Uh oh!

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

jonburdo and carljm reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
zmievsa
zmievsa suggested changes Dec 15, 2022
View reviewed changes
Comment thread Lib/os.py Outdated
return
stack = [(False, top)]
while stack:
is_result, top = stack.pop()

@zmievsa zmievsa Dec 15, 2022

Copy link
Copy Markdown
Contributor

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 don't like the is_result name for some reason. I feel like we need something better but I cannot come up with anything yet.

In case we will not find a better name, I believe we should add a comment describing what this variable signifies.

Sorry, something went wrong.

Uh oh!

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

All reactions

@jonburdo jonburdo Dec 16, 2022

Copy link
Copy Markdown
Contributor 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.

Yeah, I wasn't sure what to call it. Maybe must_yield?

Sorry, something went wrong.

Uh oh!

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

All reactions

@zmievsa zmievsa Dec 16, 2022

Copy link
Copy Markdown
Contributor

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.

Something like that would make a lot more sense, yeah. I used must_be_yielded_immediately in my PR but it's a questionable name.

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/os.py Outdated
if onerror is not None:
onerror(error)
return
stack = [(False, top)]

@zmievsa zmievsa Dec 15, 2022

Copy link
Copy Markdown
Contributor

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.

Will it make more sense to use a deque here? They are optimized for appending and popping while having the same interface.

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@carljm carljm Dec 15, 2022

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 think when append and pop occur only at the end (as in this case), performance of list and deque is basically the same. When append/pop can occur at the start, then you need deque ("double-ended queue") to avoid O(n) copy.

Sorry, something went wrong.

Uh oh!

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

AlexWaygood and jonburdo reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@jonburdo jonburdo Dec 16, 2022

Copy link
Copy Markdown
Contributor 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.

In theory I think a deque is a slightly better fit, since we're only popping and appending (at a certain point list has to reallocate right?), but I don't think there's much of a difference in this case. I changed it to a deque, but can change back to a list if that's preferred

Sorry, something went wrong.

Uh oh!

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

All reactions

@carljm carljm Dec 16, 2022

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.

Yes, list has to realloc occasionally, but proportional over-allocation means that this is amortized linear cost over a long sequence of appends (as in this use case). Deque in the same usage scenario has to repeatedly allocate new "chunks" of a fixed size, so this is also linear cost. I wrote a small microbenchmark modeling "hundreds of thousands of appends, then pop them all off" and there is no detectable performance difference on my machine. I personally don't think deque has any advantage here that makes it worth the import (which has to be inline to avoid a module-level dependency, adding some cost to every call.)

Sorry, something went wrong.

Uh oh!

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

JelleZijlstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@jonburdo jonburdo Dec 16, 2022

Copy link
Copy Markdown
Contributor 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.

Yeah, good points. I just did some rough benchmarking as well and found no real difference.

Also, I didn't find the exact cause, but adding from collections import deque to the top of Lib/os.py seemed to cause test failures under test_embed and test_site. Moving the import inside of the walk function seems to work, but at that point I agree it's not worth it.

I'll switch back to list

Sorry, something went wrong.

Uh oh!

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

All reactions
jonburdo added 5 commits December 15, 2022 21:15
@jonburdo
add unit test to ensure os.walk is not affect by recursion limit
3552d49
@jonburdo
move os._walk code to os.walk
3078ea6
@jonburdo
fix blurb to mention os.walk
f37bbe8
@jonburdo
use deque in os.walk
f26a5b8
@jonburdo
Merge branch 'main' into iterative-os-walk
955d21b
zmievsa
zmievsa suggested changes Dec 16, 2022
View reviewed changes
Comment thread Lib/test/support/__init__.py Outdated
testcase.assertRaisesRegex(TypeError, msg, tp, *args, **kwds)

@contextlib.contextmanager
def temp_recursion_limit(limit):

@zmievsa zmievsa Dec 16, 2022

Copy link
Copy Markdown
Contributor

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.

Given that it's a contextmanager, I believe the better name would be "set_recursion_limit" similar to "redirect_stdout" because the nature of a contextmanager implies that it's temporary.

Sorry, something went wrong.

Uh oh!

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

jonburdo and carljm reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
jonburdo added 4 commits December 16, 2022 10:31
@jonburdo
Merge branch 'main' into iterative-os-walk
8fbfb2e
@jonburdo
move deque import inside os.walk
ef46eda
@jonburdo
rename temp_recursion_limit to set_recursion_limit
e261b9f
@jonburdo
Merge branch 'main' into iterative-os-walk
5ec53f7
carljm
carljm approved these changes Dec 16, 2022
View reviewed changes

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

Modulo still somewhat preferring list over needing to import deque, the current version seems good to me.

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@jonburdo
switch back to list instead of deque for os.walk stack
507b650
@jonburdo

jonburdo commented Dec 17, 2022

Copy link
Copy Markdown
Contributor Author

Btw, to get a rough estimate, I did a quick performance test on my machine to compare main with this branch:

# commit d4052d835bd6f1b648d533dab8c228b6e3944651
pyperf timeit -s 'import os; p = os.path.expanduser("~")' 'list(os.walk(p))' -o main.json
# commit 507b650a7d8dc01b6115f434591238699ef8ad38
pyperf timeit -s 'import os; p = os.path.expanduser("~")' 'list(os.walk(p))' -o iterative-os-walk.json
pyperf compare_to main.json iterative-os-walk.json

This gives:

Mean +- std dev: [main] 4.04 sec +- 0.04 sec -> [iterative-os-walk] 3.76 sec +- 0.04 sec: 1.07x faster

Looks like I get around a 7% improvement.

All reactions

Sorry, something went wrong.

Uh oh!

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

@jonburdo
jonburdo requested a review from zmievsa December 17, 2022 17:13
zmievsa
zmievsa approved these changes Dec 17, 2022
View reviewed changes

@zmievsa zmievsa left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM. I am still iffy about the must_yield variable but it's the lesser evil compared to any other name I thought of.

Thank you for your hard work!

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@jonburdo

jonburdo commented Dec 17, 2022

Copy link
Copy Markdown
Contributor Author

LGTM. I am still iffy about the must_yield variable but it's the lesser evil compared to any other name I thought of.

Yeah, I couldn't think of anything better. Maybe we'll come up with a better name in the future

Thank you for your hard work!

Happy to do it! Thanks for the help :)

All reactions

Sorry, something went wrong.

Uh oh!

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

JelleZijlstra
JelleZijlstra reviewed Dec 17, 2022
View reviewed changes

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

Thanks, I have a few nits.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_os.py Outdated
os.makedirs(os.path.join(self.walk_path, *(['d'] * 50)))
with set_recursion_limit(50):
all = list(self.walk(self.walk_path))
self.assertEqual(len(all), 54)

@JelleZijlstra JelleZijlstra Dec 17, 2022

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.

Maybe also test that the return value is correct?

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@jonburdo jonburdo Dec 18, 2022

Copy link
Copy Markdown
Contributor 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.

Yeah, good idea

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/os.py Resolved

Uh oh!

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

Comment thread Lib/os.py Resolved

Uh oh!

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

Comment thread Lib/os.py Resolved

Uh oh!

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

Comment thread Lib/os.py Outdated
if topdown:
yield top, dirs, nondirs
# Traverse into sub-directories
islink, join = path.islink, path.join

@JelleZijlstra JelleZijlstra Dec 17, 2022

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.

If we're caching these functions in locals, might as well do it outside the main while loop to micro-optimize a bit more.

Sorry, something went wrong.

Uh oh!

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

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/os.py Resolved

Uh oh!

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

Comment thread Lib/os.py Resolved

Uh oh!

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

@python python deleted a comment from netlify Bot Dec 18, 2022
jonburdo and others added 5 commits December 18, 2022 17:45
@jonburdo @JelleZijlstra
Apply suggestions from code review
1c35610
Co-authored-by: Jelle Zijlstra <[email protected]>
@jonburdo
clean up os.walk comments
73138a6
@jonburdo
test correct value in test_walk_above_recursion_limit
2814cc5
@jonburdo
set islink and join before loop in os.walk
37f3cc7
@jonburdo
Merge branch 'main' into iterative-os-walk
d9c766c
@jonburdo
jonburdo requested a review from JelleZijlstra December 19, 2022 00:25
JelleZijlstra
JelleZijlstra approved these changes Dec 19, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Dec 19, 2022
@JelleZijlstra

JelleZijlstra commented Dec 19, 2022

Copy link
Copy Markdown
Member

From discussion on the issue, we shouldn't backport this, so I'm going to merge it into 3.12 (only) soon unless further feedback comes up.

Thanks for your contribution!

jonburdo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra JelleZijlstra self-assigned this Dec 19, 2022
@jonburdo jonburdo mentioned this pull request Dec 19, 2022
gh-89727: Fix os.fwalk RecursionError on deep trees #100347
Closed
@JelleZijlstra
JelleZijlstra merged commit 797edb2 into python:main Dec 19, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Dec 19, 2022
@jonburdo
jonburdo deleted the iterative-os-walk branch December 19, 2022 19:39
carljm added a commit to carljm/cpython that referenced this pull request Dec 19, 2022
@carljm
Merge branch 'main' into fastmock
a9af20a
* main:
  pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
  Docs: Don't upload CI artifacts (python#100330)
  pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
@zmievsa

zmievsa commented Dec 19, 2022

Copy link
Copy Markdown
Contributor

That was an incredibly fast merge. Congrats!

jonburdo and kalvdans reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

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

jonburdo added a commit to jonburdo/cpython that referenced this pull request Dec 20, 2022
@jonburdo
pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
55e17b8
Use a stack to implement os.walk iteratively instead of recursively to
avoid hitting recursion limits on deeply nested trees.
iritkatriel added a commit to iritkatriel/cpython that referenced this pull request Dec 28, 2022
@gvanrossum @ucodery
@carltongibson @hugovk @jonburdo @slateny @JelleZijlstra @graingert @tbwolfe @rkojedzinszky @fnesveda @pablogsal @sobolevn @ambv @sweeneyde @bizzyvinci @CAM-Gerlach @peendebak @hauntsaninja @kumaraditya303 @YuriNek0 @serhiy-storchaka @shihai1991 @amaajemyfren @brandtbucher @jollyroger182 @eendebakpt @penguin-wwy @eli-schwartz @itamaro @AlexWaygood @eric-wieser @iritkatriel @byllyfish @encukou @Fidget-Spinner @carljm @FFY00 @rhettinger @barneygale @brettcannon @AA-Turner @eryksun @seberg @illia-v @JosephSBoyle @Fraetor @MonadChains @bartbroere @glyph @Kentzo @yourlefthandman @Jackenmen
Merged main into regmachine branch (#50)
fdc73cb
* Correct CVE-2020-10735 documentation (python#100306)

* pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)

This introduces a new decorator `@inspect.markcoroutinefunction`,
which, applied to a sync function, makes it appear async to
`inspect.iscoroutinefunction()`.

* Docs: Don't upload CI artifacts (python#100330)

* pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)

Use a stack to implement os.walk iteratively instead of recursively to
avoid hitting recursion limits on deeply nested trees.

* pythongh-69929: re docs: Add more specific definition of \w (python#92015)

Co-authored-by: Jelle Zijlstra <[email protected]>

* pythongh-89051: Add ssl.OP_LEGACY_SERVER_CONNECT (python#93927)

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Christian Heimes <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Fixes python#89051

* pythongh-88211: Change lower-case and upper-case to match recommendations in imaplib docs (python#99625)

* pythongh-100348: Fix ref cycle in `asyncio._SelectorSocketTransport` with `_read_ready_cb` (python#100349)

* pythongh-99925: Fix inconsistency in `json.dumps()` error messages (pythonGH-99926)

* Clarify that every thread has its own default context in contextvars (python#99246)

* pythongh-99576: Fix cookiejar file that was not truncated for some classes (pythonGH-99616)

Co-authored-by: Łukasz Langa <[email protected]>

* pythongh-100188: Reduce misses in BINARY_SUBSCR_(LIST/TUPLE)_INT (python#100189)

Don't specialize if the index is negative.

* pythongh-99991: improve docs on str.encode and bytes.decode (python#100198)

Co-authored-by: C.A.M. Gerlach <[email protected]>

* pythongh-91081: Add note on WeakKeyDictionary behavior when deleting a replaced entry (python#91499)

Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>

* pythongh-85267: Improvements to inspect.signature __text_signature__ handling (python#98796)

This makes a couple related changes to inspect.signature's behaviour
when parsing a signature from `__text_signature__`.

First, `inspect.signature` is documented as only raising ValueError or
TypeError. However, in some cases, we could raise RuntimeError.  This PR
changes that, thereby fixing python#83685.

(Note that the new ValueErrors in RewriteSymbolics are caught and then
reraised with a message)

Second, `inspect.signature` could randomly drop parameters that it
didn't understand (corresponding to `return None` in the `p` function).
This is the core issue in python#85267. I think this is very surprising
behaviour and it seems better to fail outright.

Third, adding this new failure broke a couple tests. To fix them (and to
e.g. allow `inspect.signature(select.epoll.register)` as in python#85267), I
add constant folding of a couple binary operations to RewriteSymbolics.

(There's some discussion of making signature expression evaluation
arbitrary powerful in python#68155. I think that's out of scope. The
additional constant folding here is pretty straightforward, useful, and
not much of a slippery slope)

Fourth, while python#85267 is incorrect about the cause of the issue, it turns
out if you had consecutive newlines in __text_signature__, you'd get
`tokenize.TokenError`.

Finally, the `if name is invalid:` code path was dead, since
`parse_name` never returned `invalid`.

* pythonGH-100363: Speed up `asyncio.get_running_loop` (python#100364)

* pythonGH-100133: fix `asyncio` subprocess losing `stderr` and `stdout` output (python#100154)

* pythongh-100374: Fixed a bug in socket.getfqdn() (pythongh-100375)

* pythongh-100129: Add tests for pickling all builtin types and functions (pythonGH-100142)

* Remove unused variable from `dis._find_imports` (python#100396)

* pythongh-78878: Fix crash when creating an instance of `_ctypes.CField` (python#14837)

* pythonGH-69564: Clarify use of octal format of mode argument in help(os.chmod) (python#20621)

Co-authored-by: Kumar Aditya <[email protected]>

* pythonGH-99554: Pack location tables more effectively (pythonGH-99556)

* Correct typo in typing.py (python#100423)

In the docstring of `ParamSpec`, the name of `P = ParamSpec('P')` was
mistakenly written as `'T'`.

* pythongh-99761: Add `_PyLong_IsPositiveSingleDigit` function to check for single digit integers  (python#100064)

* pythonGH-99770: Make the correct call specialization fail kind show up in the stats (pythonGH-99771)

* pythongh-78997: fix bad rebase of moved test file (python#100424)

* pythongh-100344: Add C implementation for `asyncio.current_task` (python#100345)

Co-authored-by: pranavtbhat

* pythonGH-99554: Trim trailing whitespace (pythonGH-100435)



Automerge-Triggered-By: GH:brandtbucher

* pythongh-85432: Harmonise parameter names between C and pure-Python implementations of `datetime.time.strftime`, `datetime.datetime.fromtimestamp` (python#99993)

* pythongh-57762: fix misleading tkinter.Tk docstring (python#98837)

Mentioned as a desired change by terryjreedy on the corresponding issue,
since Tk is not a subclass of Toplevel.

* pythongh-48496: Added example and link to faq for UnboundLocalError in reference (python#93068)

* Fix typo in 3.12 What's New (python#100449)

* pythongh-76963: PEP3118 itemsize of an empty ctypes array should not be 0 (pythonGH-5576)

The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.

* pythonGH-100459: fix copy-paste errors in specialization stats (pythonGH-100460)

* pythongh-99110: Initialize `frame->previous` in init_frame to fix segmentation fault when accessing `frame.f_back` (python#100182)

* pythongh-98712: Clarify "readonly bytes-like object" semantics in C arg-parsing docs (python#98710)

* pythongh-92216: improve performance of `hasattr` for type objects (pythonGH-99979)

* pythongh-100288: Specialise LOAD_ATTR_METHOD for managed dictionaries (pythonGH-100289)

* Revert "pythongh-100288: Specialise LOAD_ATTR_METHOD for managed dictionaries (pythonGH-100289)" (python#100468)

This reverts commit c3c7848.

* pythongh-94155: Reduce hash collisions for code objects (python#100183)

* Uses a better hashing algorithm to get better dispersion and remove commutativity.

* Incorporates `co_firstlineno`, `Py_SIZE(co)`, and bytecode instructions.

* This is now the entire set of criteria used in `code_richcompare`, except for `_PyCode_ConstantKey` (which would incorporate the types of `co_consts` rather than just their values).

* pythongh-83076: 3.8x speed improvement in (Async)Mock instantiation (python#100252)

* pythongh-99482: remove `jython` compatibility parts from stdlib and tests (python#99484)

* bpo-40447: accept all path-like objects in compileall.compile_file (python#19883)

Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
Co-authored-by: Shantanu <[email protected]>

* pythonGH-100425: Improve accuracy of builtin sum() for float inputs (pythonGH-100426)

* pythongh-68320, pythongh-88302 - Allow for private `pathlib.Path` subclassing (pythonGH-31691)

Users may wish to define subclasses of `pathlib.Path` to add or modify
existing methods. Before this change, attempting to instantiate a subclass
raised an exception like:

    AttributeError: type object 'PPath' has no attribute '_flavour'

Previously the `_flavour` attribute was assigned as follows:

    PurePath._flavour        = xxx not set!! xxx
    PurePosixPath._flavour   = _PosixFlavour()
    PureWindowsPath._flavour = _WindowsFlavour()

This change replaces it with a `_pathmod` attribute, set as follows:

    PurePath._pathmod        = os.path
    PurePosixPath._pathmod   = posixpath
    PureWindowsPath._pathmod = ntpath

Functionality from `_PosixFlavour` and `_WindowsFlavour` is moved into
`PurePath` as underscored-prefixed classmethods. Flavours are removed.

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Eryk Sun <[email protected]>

* pythongh-99947: Ensure unreported errors are chained for SystemError during import (pythonGH-99946)

* Add "strict" to dotproduct(). Add docstring. Factor-out common code. (pythonGH-100480)

* pythongh-94808: improve test coverage of number formatting (python#99472)

* pythongh-100454: Start running SSL tests with OpenSSL 3.1.0-beta1 (python#100456)

* pythongh-100268: Add is_integer method to int (python#100439)

This improves the lives of type annotation users of `float` - which type checkers implicitly treat as `int|float` because that is what most code actually wants. Before this change a `.is_integer()` method could not be assumed to exist on things annotated as `: float` due to the method not existing on both types.

* pythongh-77771: Add enterabs example in sched (python#92716)

Co-authored-by: Shantanu <[email protected]>

* pythonGH-91166: Implement zero copy writes for `SelectorSocketTransport` in asyncio (python#31871)

Co-authored-by: Guido van Rossum <[email protected]>

* pythonGH-91166: Implement zero copy writes for `SelectorSocketTransport` in asyncio (python#31871)

Co-authored-by: Guido van Rossum <[email protected]>

* Misc Itertools recipe tweaks (pythonGH-100493)

* pythongh-100357: Convert several functions in `bltinsmodule` to AC (python#100358)

* Remove wrong comment about `repr` in `test_unicode` (python#100495)

* pythongh-99908: Tutorial: Modernize the 'data-record class' example (python#100499)

Co-authored-by: Alex Waygood <[email protected]>

* pythongh-100474: Fix handling of dirs named index.html in http.server (pythonGH-100475)



If you had a directory called index.html or index.htm within a directory, it would cause http.server to return a 404 Not Found error instead of the directory listing. This came about due to not checking that the index was a regular file.

I have also added a test case for this situation.

Automerge-Triggered-By: GH:merwok

* pythongh-100287: Fix unittest.mock.seal with AsyncMock (python#100496)

* pythongh-99535: Add test for inheritance of annotations and update documentation (python#99990)

* pythongh-100428: Make float documentation more accurate (python#100437)

Previously, the grammar did not accept `float("10")`.
Also implement mdickinson's suggestion of removing the indirection.

* [Minor PR] Quotes in documentation changed into code blocks (python#99536)

Minor formatting fix in documentation

Co-authored-by: Shantanu <[email protected]>

* pythongh-100472: Fix docs claim that compileall parameters could be bytes (python#100473)

* pythongh-100519: simplification to `eff_request_host` in cookiejar.py (python#99588)

`IPV4_RE` includes a `.`, and the `.find(".") == -1` included here is already testing to make sure there's no dot, so this part of the expression is tautological. Instead use more modern `in` syntax to make it clear what the check is doing here. The simplified implementation more clearly matches the wording in RFC 2965.

Co-authored-by: hauntsaninja <[email protected]>

* pythongh-99308: Clarify re docs for byte pattern group names (python#99311)

* pythongh-92446: Improve argparse choices docs; revert bad change to lzma docs (python#94627)

Based on the definition of the collections.abc classes, it is more accurate to use "sequence" instead of "container" when describing argparse choices.

A previous attempt at fixing this in python#92450 was mistaken; this PR reverts that change.

Co-authored-by: Shantanu <[email protected]>

* Fix name of removed `inspect.Signature.from_builtin` method in 3.11.0a2 changelog (python#100525)

* pythongh-100520: Fix `rst` markup in `configparser`  docstrings (python#100524)

* pythongh-99509: Add `__class_getitem__` to `multiprocessing.queues.Queue` (python#99511)

* pythongh-94603: micro optimize list.pop (pythongh-94604)

* Remove `NoneType` redefinition from `clinic.py` (python#100551)

* pythongh-100553: Improve accuracy of sqlite3.Row iter test (python#100555)

* pythonGH-98831: Modernize a ton of simpler instructions (python#100545)

* load_const and load_fast aren't families for now
* Don't decref unmoved names
* Modernize GET_ANEXT
* Modernize GET_AWAITABLE
* Modernize ASYNC_GEN_WRAP
* Modernize YIELD_VALUE
* Modernize POP_EXCEPT (in more than one way)
* Modernize PREP_RERAISE_STAR
* Modernize LOAD_ASSERTION_ERROR
* Modernize LOAD_BUILD_CLASS
* Modernize STORE_NAME
* Modernize LOAD_NAME
* Modernize LOAD_CLASSDEREF
* Modernize LOAD_DEREF
* Modernize STORE_DEREF
* Modernize COPY_FREE_VARS (mark it as done)
* Modernize LIST_TO_TUPLE
* Modernize LIST_EXTEND
* Modernize SET_UPDATE
* Modernize SETUP_ANNOTATIONS
* Modernize DICT_UPDATE
* Modernize DICT_MERGE
* Modernize MAP_ADD
* Modernize IS_OP
* Modernize CONTAINS_OP
* Modernize CHECK_EXC_MATCH
* Modernize IMPORT_NAME
* Modernize IMPORT_STAR
* Modernize IMPORT_FROM
* Modernize JUMP_FORWARD (mark it as done)
* Modernize JUMP_BACKWARD (mark it as done)

Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Co-authored-by: Jeremy Paige <[email protected]>
Co-authored-by: Carlton Gibson <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Jon Burdo <[email protected]>
Co-authored-by: Stanley <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Brad Wolfe <[email protected]>
Co-authored-by: Richard Kojedzinszky <[email protected]>
Co-authored-by: František Nesveda <[email protected]>
Co-authored-by: Pablo Galindo Salgado <[email protected]>
Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Co-authored-by: Dennis Sweeney <[email protected]>
Co-authored-by: Bisola Olasehinde <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Shantanu <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Dominic Socular <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Hai Shi <[email protected]>
Co-authored-by: amaajemyfren <[email protected]>
Co-authored-by: Brandt Bucher <[email protected]>
Co-authored-by: david-why <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: penguin_wwy <[email protected]>
Co-authored-by: Eli Schwartz <[email protected]>
Co-authored-by: Itamar Ostricher <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Eric Wieser <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
Co-authored-by: Bill Fisher <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Carl Meyer <[email protected]>
Co-authored-by: Filipe Laíns <[email protected]>
Co-authored-by: Raymond Hettinger <[email protected]>
Co-authored-by: Barney Gale <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Eryk Sun <[email protected]>
Co-authored-by: Sebastian Berg <[email protected]>
Co-authored-by: Illia Volochii <[email protected]>
Co-authored-by: JosephSBoyle <[email protected]>
Co-authored-by: James Frost <[email protected]>
Co-authored-by: MonadChains <[email protected]>
Co-authored-by: Bart Broere <[email protected]>
Co-authored-by: Glyph <[email protected]>
Co-authored-by: hauntsaninja <[email protected]>
Co-authored-by: Ilya Kulakov <[email protected]>
Co-authored-by: Guy Yagev <[email protected]>
Co-authored-by: Jakub Kuczys <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm approved these changes
@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
+1 more reviewer
@zmievsa zmievsa zmievsa approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@JelleZijlstra JelleZijlstra

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.

6 participants

@jonburdo @bedevere-bot @careljonkhout @carljm @JelleZijlstra @zmievsa
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.