Skip to content

Conversation

@viiccwen
Copy link
Contributor

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

  • Bug fix
  • New feature
  • Refactoring
  • Documentation
  • Test
  • CI/CD pipeline
  • Other

Breaking Changes

  • Yes
  • No

Detailed Changes

Code Refactoring (src/lib.rs)

  1. 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
  2. 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)

  1. 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
  2. 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

- 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.
…scenarios

- Added fixtures for creating QdpEngine instances with default and float64 precision.
- Introduced new tests for encoding from NumPy arrays and pathlib.Path objects.
- Updated existing tests to utilize the new fixtures.
@viiccwen viiccwen changed the title Refactor 1d2d shape matching [QDP] Refactor encode() method into helper functions with tests Jan 12, 2026
@viiccwen
Copy link
Contributor Author

cc @400Ping, @guan404ming 🙌

@400Ping
Copy link
Contributor

400Ping commented Jan 14, 2026

Please resolve conflicts

@ryankert01 ryankert01 self-requested a review January 14, 2026 19:03
- 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.
@rich7420
Copy link
Contributor

plz resolve the pre-commit error

@ryankert01
Copy link
Member

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

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.

4 participants