Conversation
…r than the STAR's own Reading a saved configuration back needs more than `json.load`: JSON loses a tuple, a dict's key type and a date, and what each field is declared to hold is what puts all three back. That walk, and the `to_jsonable` that writes it, were defined on the STAR driver's configuration module, so any other driver wanting to save and load its own configurations had to import from the STAR to get them. They move to `pylabrobot.utils.configuration_json` unchanged. The STAR's configuration module reads with them as before, and the five places that wrote with `to_jsonable` import it from its new home. No behaviour changes: the same walk, over the same declared types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| def to_jsonable(value: Any) -> Any: | ||
| """The value as JSON holds it. | ||
|
|
||
| Args: | ||
| value: what to convert - a configuration, or anything one holds. | ||
|
|
||
| Returns: | ||
| The same value in types `json.dump` accepts. | ||
| """ | ||
| if dataclasses.is_dataclass(value) and not isinstance(value, type): | ||
| return { | ||
| field.name: to_jsonable(getattr(value, field.name)) for field in dataclasses.fields(value) | ||
| } | ||
| if isinstance(value, datetime.date): | ||
| return value.isoformat() | ||
| if isinstance(value, (list, tuple)): | ||
| return [to_jsonable(item) for item in value] | ||
| if isinstance(value, dict): | ||
| # Keys are written as text because JSON has no other kind. What they were is on the field. | ||
| return {str(key): to_jsonable(item) for key, item in value.items()} | ||
| return value | ||
|
|
There was a problem hiding this comment.
we already have a serialize function for this, why not just use that?
|
this entire PR is duplicating the serializer instead of extending it with datetime |
BioCam
changed the title
pylabrobot.utils.configuration_jsonutils.configuration_json: hold the configuration JSON walk once, out of the STAR driver
The changelog is written at the next release, not per change. What this moves, and that `to_jsonable` is imported from a new home, belongs in the PR description. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@rickwierenga - I think it's a different job. Happy to fold it into |
|
it needs a type for classes, not for ordinary data. the job is exactly the same, turn python data into json-compatible dicts and back this diff should be approx 4 lines in serializer.py |
|
I tried it: the write side is 3 lines, but the read side isn't. Neither can come back from the json alone - |
2 participants
Reading a saved configuration back needs more than
json.load: JSON loses a tuple, a dict's key type and a date, and what each field is declared to hold is what puts all three back. That walk, and theto_jsonablethat writes it, are defined on the STAR driver's configuration module, so a driver that wants to save and load its own configurations has to import from the STAR to get them.They move to
pylabrobot.utils.configuration_json, unchanged.to_jsonable, and the walk that reads a value back against the type its field declares:Optional, fixed- and variable-length tuples, lists, dicts including the int keys JSON writes as text, dates, and nested dataclasses. A name the class no longer has is left out, so a file written by a driver that has since dropped a field still loads.star/driver/configuration.pyloses the two definitions and reads with the shared ones instead: 79 lines out, 2 in.to_jsonable,star/driver/master.pyand four test modules, import it from its new home.Behaviour: the same walk over the same declared types. Nothing is written or read differently, and no configuration file that loaded before stops loading.
Tests:
configuration_json_tests.pycomes across with the module. The full suite passes (3410 passed, 4 skipped, 952 subtests), under pytest 9.0.3 and 9.1.1 both, since the two differ oncaplog.make lint,make format-checkandtyposare clean, and every tracked source parses under Python 3.9.make typecheckreports only the nine errors inli_cor/odyssey/odyssey_tests.pythat are already onmain, in a file this branch does not touch.🤖 Generated with Claude Code