Fix pickling for the C extension - #838
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #838 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 2 2
Lines 383 642 +259
Branches 9 37 +28
==========================================
+ Hits 383 642 +259
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:
|
|
The functional CI matrix, Codecov, documentation, lint, and pre-commit checks are green. I also reproduced the repository's MyPy pre-commit invocation locally ( The remaining Coveralls statuses report 93.103% and a 0.1% decrease, which does not match the generated local Cobertura report or the 100% Codecov test coverage report. I have not added unrelated changes just to mask that external coverage discrepancy. |
Used OpenAI Codex (GPT-5) to inspect the Coveralls MyPy report and make the new pickle tests type-precise. Reviewed the diff, ran MyPy with Cobertura output, and ran the pure-Python test module.
for more information, see https://pre-commit.ci
|
I tightened the new pickle tests so the MyPy report treats the added assertions as typed code (commit 7025fa5, with pre-commit.ci's follow-up 0a36b9). Local MyPy passes with 96.20% type-check coverage, and the pure-Python test module passes 116 tests. The existing full CI matrix and Codecov report are green; the refreshed CI run is queued now. I also marked the PR ready for review. |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (12): Last reviewed commit: "Handle mangled and mixed slot state" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed the serialization concerns in |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Updated in c275abb to preserve subclass pickle state for instance dictionaries, slots, and custom getstate/setstate implementations. The pure-Python fallback unpickle path now has a regression test as well. Local full test suite passes (120 tests), plus Ruff, Mypy, and Cython validation. Please take another look when convenient. |
for more information, see https://pre-commit.ci
|
The latest commit 4621ce3 also has a fully green CI run: all 51 required checks passed, including Codecov, Coveralls, the full test matrix, all tested-architecture wheels, lint, and Greptile. The remaining step is maintainer review. |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Latest commit 844120c addresses the remaining slot-state feedback, including mangled private slots and mixed dict/slot restoration. All review threads are resolved, and the required CI, CodeQL, coverage, docs, lint, and pre-commit checks are green. The PR is ready for maintainer review whenever convenient. Thanks! |
2 participants
What do these changes do?
Add an explicit pickle reducer to the Cython implementation so the atomic
frozen state is serialized through a small reconstruction helper.
Are there changes in behavior for the user?
Yes. Pickling
FrozenListinstances works when the C extension is enabled,for both frozen and unfrozen instances.
Related issue number
Fixes #834
Checklist
CONTRIBUTORS.txtCHANGESfolderAgent run details
ruff check frozenlist testspassed.flake8 frozenlist testspassed.cython-lint --no-pycodestyle frozenlist/_frozenlist.pyxpassed.git diff --checkpassed.Drafted with OpenAI Codex; human review is required before marking this PR ready.