Skip to content
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.

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

feat: serialize named tuples as dicts #837

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

maffettone
Copy link
Contributor

Take advantage of dependent PR in Tiled to serialize NamedTuple like objects as dictionaries.

Description

Add function to be consumed as default serialization.

Motivation and Context

This default functionality was removed from orjson in version 3.10, and causes problems with Bluesky use (e.g., hklpy).

How Has This Been Tested?

@maffettone
Copy link
Contributor Author

Tagging @ambarb for visibility.

@maffettone maffettone force-pushed the feat-safe-json-dump-named-tuples branch from 4ad2427 to e7dfc56 Compare February 20, 2025 13:34
@maffettone maffettone force-pushed the feat-safe-json-dump-named-tuples branch from f1e2a66 to 3609f9c Compare February 20, 2025 13:57
@danielballan
Copy link
Member

I appreciate the effort to isolate and target this fix. Unfortunately, as we look toward migrating the facility off of mongo_normalized during 2025, this fix would be short-lived.

At the moment, databroker registers a custom endpoint with the Tiled FastAPI Router, POST /documents/{path}, which you are customizing here, and blueksy_tiled_plugins.CatalogOfBlueskyRuns.post_document calls it. But in the future (and already at HEX) the bluesky.callbacks.tiled_writer.TiledWriter will be used, and it will send metadata from the start document at the vanilla POST /metadata endpoint.

Other options:

  • Make Tiled tolerate namedtuple and perhaps other by default. (I don't love this. I think orjson is right that there is no unambigous JSON serialization of namedtuple.)
  • Merge this fix, and add tolerant JSON serialization into TiledWriter as well. Tiled would need a way to expose this in tiled.client.container.Container.new().
  • In the Device, use a serializable type.

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.

3 participants