Repository navigation
Stable code identifier pickling - #40711
kristynsmith wants to merge 24 commits into
Conversation
Replace config assignment with dataclasses.replace for stability.
Refactor code object handling and improve normalization for hashing.
Added new test cases for lambda functions and decorators in code_object_pickler.
Refactor code object search logic to handle local code objects and update lambda handling. Replace lambda-specific functions with more general object handling functions.
Added tests for handling same-named nested functions with different and same arguments in code object pickler.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40711 +/- ##
==========================================
- Coverage 80.04% 79.82% -0.23%
==========================================
Files 490 494 +4
Lines 82916 83594 +678
==========================================
+ Hits 66373 66731 +358
- Misses 16543 16863 +320
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Added tests for handling complex nested functions and edge cases in code object pickling.
Refactor function names for clarity and consistency in tests.
There was a problem hiding this comment.
📋 Review Summary
This PR resolves several edge cases in code_object_pickler around duplicate nested function names, zero-argument signatures, identical bytecode instructions with differing constants/closures, module prefix resolution, and shared decorator wrappers. The logic changes are clean, accurate, and well-covered by new unit tests.
🔍 General Feedback
- The generalization of
_search_localsand the regex patterns (_OBJECT_WITH_ARGS_PATTERNand_OBJECT_WITH_HASH_PATTERN) cleanly handles both<lambda>and duplicate named local functions. - Including normalized
co_consts,co_names,co_varnames,co_freevars, andco_cellvarsin_create_bytecode_hashproperly prevents hash collisions whenco_codealone is identical. - One minor issue in
_normalize_const:frozensetiteration order is non-deterministic across processes, so its elements should be sorted when normalizing for hashing.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Assigning reviewers: R: @tvalentyn for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Fixes several edge cases in
code_object_picklerwhen generating and resolving stable code object identifiers:deffunctions have the same name (e.g. defined acrossif/elsebranches) by applying the same signature and bytecode-hash disambiguation in_search_locals(formerly_search_lambda) to any duplicate code object name inco_consts, and updating_OBJECT_WITH_ARGS_PATTERNand_OBJECT_WITH_HASH_PATTERNto match named functions in addition to<lambda>._OBJECT_WITH_ARGS_PATTERNto match empty argument tuples().co_codebytecode instructions (e.g. referencing different closure variables or constants) by includingco_consts,co_names,co_varnames,co_freevars, andco_cellvarsin_create_bytecode_hash.from pkg.submod import submodinpkg/__init__.py) by resolving the longest matching module prefix inget_code_from_identifierbefore traversing attributes.None(falling back to default pickling) instead of raisingValueError('Qual name parts too long')when a branch does not match the full__qualname__.