Skip to content

make return types picklable#99

Merged
j6k4m8 merged 1 commit into
aplbrain:masterfrom
q-rosiebloxsom:make-results-picklable
Jun 30, 2026
Merged

make return types picklable#99
j6k4m8 merged 1 commit into
aplbrain:masterfrom
q-rosiebloxsom:make-results-picklable

Conversation

@q-rosiebloxsom

Copy link
Copy Markdown
Contributor

This is a quick one to help anyone using GrandCypher results downstream.

Fix pickling of query result keys (AttributeRef, IDRef, EntityRef)

Problem

The str subclasses in grandcypher/types.py that are used as result dictionary keys don't survive pickle round-trips. This breaks any downstream code that serialises GrandCypher query results — e.g. PySpark map(), multiprocessing, joblib, or caching.

AttributeRef — crashes on unpickle:

AttributeRef.__new__ takes two arguments (entity_name, attribute), but pickle's default str reconstruction only passes the single string value:

>>> pickle.loads(pickle.dumps(AttributeRef('a', 'name')))
TypeError: AttributeRef.__new__() missing 1 required positional argument: 'attribute'

IDRef — silently corrupts on unpickle:

IDRef.__new__ wraps its argument as f"ID({entity_name})". Pickle reconstructs with str(self) which is already "ID(a)", so __new__ wraps it again:

>>> pickle.loads(pickle.dumps(IDRef('a')))
'ID(ID(a))'

Root cause

Python's str pickle path calls cls.__new__(cls, str_value) on unpickle. This works when __new__ takes exactly one argument matching the string value (as with EntityRef), but breaks when:

  • __new__ takes extra arguments (AttributeRef)
  • __new__ transforms the argument (IDRef prepends "ID(")

Fix

Added __getnewargs__ to all three ref classes (AttributeRef, IDRef, EntityRef). This method tells pickle which arguments to pass to __new__ during reconstruction, so the original constructor arguments are preserved across serialisation.

Changes

  • grandcypher/types.py — added __getnewargs__ to EntityRef, AttributeRef, and IDRef
  • grandcypher/test_types.py (new) — pickle round-trip tests for each ref class individually, plus an end-to-end test that pickles the full result dict from a GrandCypher.run() query

@codspeed-hq

codspeed-hq Bot commented Jun 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 99 untouched benchmarks


Comparing q-rosiebloxsom:make-results-picklable (3782fa0) with master (18a2af5)

Open in CodSpeed

@j6k4m8 j6k4m8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great feature!

@j6k4m8
j6k4m8 merged commit 2eba492 into aplbrain:master Jun 30, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants