Remove to_bytes / from_bytes from non-top-level Python wrappers#845
Merged
Remove to_bytes / from_bytes from non-top-level Python wrappers#845
Conversation
Drop bytes serialization from component-level Python wrappers (`Function`, `Linear`, `Quadratic`, `Polynomial`, `Parameter`, `NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction`, `DecisionVariable`, `EvaluatedDecisionVariable`, `SampledDecisionVariable`). Bytes serialization remains on the top-level container types (`Instance`, `ParametricInstance`, `Solution`, `SampleSet`) and the cross-evaluate DTOs (`State`, `Samples`, `Parameters`). The component bytes round-trip is lossy with respect to metadata that lives on the parent container, so exposing it from individual elements invites silent data loss. Drop the API surface ahead of the metadata-storage refactor that makes the lossiness explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows the Python (PyO3) binding surface by removing to_bytes / from_bytes from non-top-level “component” wrapper types, keeping bytes serialization only on top-level container types and cross-evaluate DTOs to avoid metadata-lossy round-trips.
Changes:
- Removed
to_bytes/from_bytesmethods from component-level wrappers (e.g.,DecisionVariable,Function,Linear,NamedFunction, evaluated/sampled variants). - Regenerated Python stubs (
__init__.pyi) and API reference JSON to reflect the updated public surface. - Removed Python tests that exercised now-deleted component-level bytes round-trips.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/ommx/src/decision_variable.rs | Removes bytes serialization APIs from DecisionVariable wrapper. |
| python/ommx/src/evaluated_decision_variable.rs | Removes bytes serialization APIs from EvaluatedDecisionVariable wrapper. |
| python/ommx/src/sampled_decision_variable.rs | Removes bytes serialization APIs from SampledDecisionVariable wrapper. |
| python/ommx/src/function.rs | Removes bytes serialization APIs from Function wrapper. |
| python/ommx/src/linear.rs | Removes bytes serialization APIs from Linear wrapper. |
| python/ommx/src/quadratic.rs | Removes bytes serialization APIs from Quadratic wrapper. |
| python/ommx/src/polynomial.rs | Removes bytes serialization APIs from Polynomial wrapper. |
| python/ommx/src/parameter.rs | Removes bytes serialization APIs from Parameter wrapper. |
| python/ommx/src/named_function.rs | Removes bytes serialization APIs from NamedFunction wrapper. |
| python/ommx/src/evaluated_named_function.rs | Removes bytes serialization APIs from EvaluatedNamedFunction wrapper. |
| python/ommx/src/sampled_named_function.rs | Removes bytes serialization APIs from SampledNamedFunction wrapper. |
| python/ommx/ommx/_ommx_rust/init.pyi | Updates generated stubs to remove the deleted methods from affected classes. |
| python/ommx-tests/tests/test_named_function.py | Removes NamedFunction bytes round-trip test that no longer applies. |
| docs/api/api_reference.json | Updates generated API reference to remove the deleted methods from affected classes. |
…ytes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
termoshtt
added a commit
that referenced
this pull request
Apr 28, 2026
Same treatment as 1.x — the 2.x release notes pin code samples to API shapes that no longer compile under v3 (e.g. property-style `*_df` accessors, the bytes-on-element-types methods removed in #845). Build the pages but don't try to execute them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
termoshtt
added a commit
that referenced
this pull request
Apr 30, 2026
## Summary - Move `name` / `subscripts` / `parameters` / `description` off `NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction` onto a new `NamedFunctionMetadataStore` SoA store, kept as a sibling field on `Instance` / `ParametricInstance` / `Solution` / `SampleSet`. Same snapshot-model PyO3 wrappers and `WithMetadata`-based pandas rendering as PR #843 (which did this for `Constraint` / `DecisionVariable`). - Parse boundary moves to per-collection: `Vec<v1::NamedFunction>::Parse` returns `(BTreeMap, NamedFunctionMetadataStore)`; per-element helpers (`named_function_to_v1`, `evaluated_named_function_to_v1`, `sampled_named_function_to_v1`) reattach metadata at emit. - `Instance::evaluate` / `evaluate_samples` thread the host store onto the produced `Solution` / `SampleSet`. `SampleSet::get` carries the store onto each per-sample `Solution`. - Element-level `to_bytes` / `from_bytes` removed from `NamedFunction` / `EvaluatedNamedFunction` / `SampledNamedFunction` — same rationale as #845: a per-element wire round-trip can no longer carry host-stored metadata, so the methods became misleading rather than safe. - METADATA_STORAGE_V3.md flipped \`NamedFunction\` from \"deferred — separate PR\" to \"landed (this PR)\". ## End state After this PR the v3 SoA metadata story is uniform across all four metadata-bearing element types (decision variables, regular / indicator / one-hot / sos1 constraints, named functions). The only remaining deferred item in METADATA_STORAGE_V3.md is the two-mode Standalone / Attached wrapper design for write-through metadata mutation. ## Test plan - [x] \`cargo test -p ommx --lib\` — 475 passed (incl. new \`test_instance_roundtrip_preserves_named_function_metadata\` regression test) - [x] \`task python:test\` — all adapter + ommx-tests pass; lint + typecheck clean - [x] \`cargo insta test --accept\` for the 5 \`instance::logical_memory\` snapshots that gained the new \`Instance.named_function_metadata;…\` rows - [x] Format / clippy clean on changed files --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops bytes serialization (
to_bytes/from_bytes) from component-level Python wrappers. Top-level container types and cross-evaluate DTOs keep bytes serialization unchanged.This is a Python SDK breaking change, split out from #843 so the larger metadata-storage refactor can land separately. The Rust SDK is unchanged in this PR — only the PyO3 binding surface narrows.
Affected types
Removed
to_bytes/from_bytes:Function,Linear,Quadratic,PolynomialParameterNamedFunction,EvaluatedNamedFunction,SampledNamedFunctionDecisionVariable,EvaluatedDecisionVariable,SampledDecisionVariableKept (top-level + cross-evaluate DTOs):
Instance,ParametricInstance,Solution,SampleSetState,Samples,ParametersRationale
These methods originally existed because the pre-v3 Python SDK had its own protobuf-based wrapper layer, so every value crossing the Python ↔ Rust boundary had to be serialized —
to_bytes/from_byteswas the bridge. With v3's switch to direct PyO3 re-exports the boundary is gone, and element-level bytes round-trips no longer serve a purpose. The upcoming metadata-storage redesign (#843 lineage) would only inflate the maintenance cost of keeping them, so they're retired now.Test plan
task python:stubgenregenerated stubs anddocs/api/api_reference.jsontask python:testpasses (ommx core + highs / pyscipopt / python-mip / openjij adapters)task formatclean🤖 Generated with Claude Code