[QDP] Refactor `encode()` method into helper functions with tests #814

viiccwen · 2026-01-12T16:45:52Z

Purpose of PR

Refactor QdpEngine.encode() method to improve code maintainability and readability by splitting the monolithic 150+ line method into smaller and focused helper methods.

This PR also adds/refactors tests to ensure all adjust code paths are covered.

Related Issues or PRs

comment

Changes Made

Breaking Changes

Yes
No

Detailed Changes

Code Refactoring (`src/lib.rs`)

Split encode() method into focused helper methods:
- encode_from_numpy() - Handles NumPy array input (1D and 2D)
- encode_from_pytorch() - Handles PyTorch tensor input (1D and 2D)
- encode_from_list() - Handles Python list input
- Main encode() method now acts as a router, delegating to appropriate handlers
Benefits:
- Improved code readability (each method has a single responsibility)
- Easier to maintain and modify individual input type handlers
- Better testability (each helper method can be tested independently)
- Reduced code duplication

Test Improvements (`tests/test_bindings.py`)

Added 3 tests for refactored paths:
- test_encode_numpy_array() - Tests NumPy array encoding (1D and 2D)
- test_encode_pytorch_tensor() - Tests PyTorch tensor encoding (1D and 2D)
- test_encode_pathlib_path() - Tests pathlib.Path object input
Code quality improvements:
- Extracted test data constants (TEST_DATA_1D, NUM_QUBITS, SAMPLE_SIZE)
- Added pytest fixtures (engine, engine_float64) to reduce duplication
- Simplified test functions by using fixtures and constants

Testing

All existing tests pass
New tests added for all refactored code paths
Tests verify both 1D and 2D inputs for NumPy arrays and PyTorch tensors
Tests verify pathlib.Path object handling

Checklist

Added or updated unit tests for all changes
Added or updated documentation for all changes
Successfully built and ran all unit tests or manual tests locally
PR title follows "MAHOUT-XXX: Brief Description" format (if related to an issue)
Code follows ASF guidelines

viiccwen · 2026-01-12T16:48:39Z

cc @400Ping, @guan404ming 🙌

400Ping · 2026-01-14T08:06:38Z

Please resolve conflicts

rich7420 · 2026-01-15T03:35:40Z

plz resolve the pre-commit error

ryankert01 · 2026-01-16T06:42:02Z

please resolve the conflict and re-think the solution with newly introduced testing refactor if needed

viiccwen · 2026-01-18T01:54:14Z

please resolve the conflict and re-think the solution with newly introduced testing refactor if needed

Sure, I'll kick off today or tomorrow!

guan404ming · 2026-01-20T15:30:19Z

Hey @viiccwen, how this going? Feel free let me know if you need any help. Thanks!

viiccwen · 2026-01-20T15:33:52Z

replying here!
#881 (comment)

I'll dive into refactor encode (qdp/qdp-python/src/lib.rs) after this PR merged.

Edit: bcz this file updated frequently.

guan404ming · 2026-01-20T15:35:06Z

Make sense, thanks.

ryankert01

need to resolve conflicts

- Introduced dedicated methods `encode_from_numpy` and `encode_from_pytorch` to streamline the encoding process for NumPy arrays and PyTorch tensors, respectively. - Improved error handling for unsupported shapes in both encoding methods. - Simplified the main encoding logic by delegating to these new methods.

Replace deprecated PyReadonlyArrayDyn with dimension-specific types PyReadonlyArray1 and PyReadonlyArray2 for better type safety and compatibility with newer pyo3-numpy versions.

rich7420

@viiccwen thanks for the update!
Tests all passed locally.
left some comments:

rich7420 · 2026-01-23T06:42:38Z

qdp/qdp-python/src/lib.rs

+    /// CUDA device ID from DLPack metadata.
+    /// Currently unused but kept for potential future device validation or multi-GPU support.
+    #[allow(dead_code)]
+    device_id: i32,


Since this is a dead code but may be used in the future.
I think maybe we could add a defensive assertion to verify DLPack device_id matches the validated device_id.

IMO, it could be a follow-up. WDYT?
bcz this PR only deals with refactoring encode path & adding missing tests.

no problem, plz open a issue.

qdp/qdp-python/src/lib.rs

viiccwen · 2026-01-23T09:46:29Z

another conflict 💀

….py`

viiccwen · 2026-01-23T16:18:46Z

Thanks for reviewing! 🙌

viiccwen changed the title ~~Refactor 1d2d shape matching~~ [QDP] Refactor encode() method into helper functions with tests Jan 12, 2026

ryankert01 self-requested a review January 14, 2026 19:03

guan404ming added this to the Qumat 0.5.0 milestone Jan 20, 2026

ryankert01 reviewed Jan 21, 2026

View reviewed changes

viiccwen added 4 commits January 22, 2026 10:23

chore: remove unused import

e5fcba7

fix(python): replace PyReadonlyArrayDyn with PyReadonlyArray1/2

e3a1c54

Replace deprecated PyReadonlyArrayDyn with dimension-specific types PyReadonlyArray1 and PyReadonlyArray2 for better type safety and compatibility with newer pyo3-numpy versions.

refactor(tests): refactor test_bindings.py

5a22fa0

viiccwen force-pushed the refactor-1d2d-shape-matching branch from 496fb7c to 5a22fa0 Compare January 22, 2026 11:09

rich7420 reviewed Jan 23, 2026

View reviewed changes

viiccwen added 2 commits January 23, 2026 07:07

feat(validation): add shape validation for arrays and tensors

aa89eb6

test: add 3D tensor shape validation testing

187b513

viiccwen mentioned this pull request Jan 23, 2026

[Feature] Add defensive assertion for DLPack device_id #920

Open

viiccwen added 2 commits January 23, 2026 09:57

merge from main branch

12222fd

Remove commented-out section for IQP Encoding Tests in `test_bindings…

8c68329

….py`

guan404ming approved these changes Jan 23, 2026

View reviewed changes

guan404ming merged commit 11db508 into apache:main Jan 23, 2026
6 checks passed

[QDP] Refactor encode() method into helper functions with tests #814

[QDP] Refactor encode() method into helper functions with tests #814

Conversation

viiccwen commented Jan 12, 2026

Purpose of PR

Related Issues or PRs

Changes Made

Breaking Changes

Detailed Changes

Code Refactoring (src/lib.rs)

Test Improvements (tests/test_bindings.py)

Testing

Checklist

Uh oh!

viiccwen commented Jan 12, 2026

Uh oh!

400Ping commented Jan 14, 2026

Uh oh!

rich7420 commented Jan 15, 2026

Uh oh!

ryankert01 commented Jan 16, 2026

Uh oh!

viiccwen commented Jan 18, 2026

Uh oh!

guan404ming commented Jan 20, 2026

Uh oh!

viiccwen commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guan404ming commented Jan 20, 2026

Uh oh!

ryankert01 left a comment

Choose a reason for hiding this comment

Uh oh!

rich7420 left a comment

Choose a reason for hiding this comment

Uh oh!

rich7420 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

viiccwen Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

rich7420 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

viiccwen commented Jan 23, 2026

Uh oh!

Uh oh!

viiccwen commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[QDP] Refactor `encode()` method into helper functions with tests #814

[QDP] Refactor `encode()` method into helper functions with tests #814

Code Refactoring (`src/lib.rs`)

Test Improvements (`tests/test_bindings.py`)

viiccwen commented Jan 20, 2026 •

edited

Loading