Skip to content

feat(python): add Python bindings and wrappers #2

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

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

hakula139
Copy link
Owner

No description provided.

@hakula139 hakula139 self-assigned this Aug 10, 2025
@Copilot Copilot AI review requested due to automatic review settings August 10, 2025 11:14

This comment was marked as off-topic.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive Python bindings for C++ features, providing a modern Python wrapper that showcases Python 3.13 features like pattern matching, enhanced error handling, and improved type hints. The bindings cover timing, shapes, random generation, memory management, exception handling, containers, and algorithms.

Key Changes

  • Added modern Python wrappers with type hints and pattern matching for all C++ modules
  • Implemented comprehensive test suites following C++ test patterns for Python functionality
  • Created detailed examples demonstrating real-world usage of the Python bindings

Reviewed Changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 6 comments.

File Description
tests/CMakeLists.txt Updated comment from "Check if" to "Ensure Catch2 is available"
python/tests/* Complete test suite covering all modules with comprehensive unit and integration tests
python/src/* Modern Python wrappers for timing, shapes, random, memory, exceptions, containers, and algorithms
python/examples/* Detailed example scripts demonstrating usage patterns and best practices


assert ns > 0
assert us > 0
assert ns >= us # ns should be >= us (1000x conversion)
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion logic is incorrect. Nanoseconds should be greater than or equal to microseconds, but the relationship should be ns >= us * 1000, not ns >= us. A value of 1000 ns equals 1 us, so ns values should typically be much larger than us values.

Suggested change
assert ns >= us # ns should be >= us (1000x conversion)
assert ns >= us * 1000 # ns should be >= us * 1000 (1000x conversion)

Copilot uses AI. Check for mistakes.

assert ns > 0
assert us > 0
assert ns >= us # ns should be >= us (1000x conversion)
assert us >= ms # us should be >= ms (1000x conversion)
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion logic is incorrect. Microseconds should be greater than or equal to milliseconds, but the relationship should be us >= ms * 1000, not us >= ms. A value of 1000 us equals 1 ms, so us values should typically be much larger than ms values.

Suggested change
assert us >= ms # us should be >= ms (1000x conversion)
assert us >= ms * 1000 # us should be >= ms * 1000 (1000x conversion)

Copilot uses AI. Check for mistakes.

assert us > 0
assert ns >= us # ns should be >= us (1000x conversion)
assert us >= ms # us should be >= ms (1000x conversion)
assert ms >= s # ms should be >= s (1000x conversion)
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion logic is incorrect. Milliseconds should be greater than or equal to seconds, but the relationship should be ms >= s * 1000, not ms >= s. A value of 1000 ms equals 1 s, so ms values should typically be much larger than s values.

Suggested change
assert ms >= s # ms should be >= s (1000x conversion)
assert ms >= s * 1000 # ms should be >= s * 1000 (1000x conversion)

Copilot uses AI. Check for mistakes.

print(f' - Average area: {comparison["average_area"]:.2f}')
print(f' - Largest shape by area: {comparison["largest_by_area"].get_name()}')
print(f' - Smallest shape by area: {comparison["smallest_by_area"].get_name()}')
print(f' - Most efficient shape: {comparison["most_efficient"].get_name()}')
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code attempts to access a 'most_efficient' key from the comparison dictionary, but the compare_shapes function in shapes.py does not return this key. Only 'largest_by_area', 'smallest_by_area', 'average_area', etc. are returned.

Suggested change
print(f' - Most efficient shape: {comparison["most_efficient"].get_name()}')
# Compute the most efficient shape manually
most_efficient_shape = max(shapes, key=lambda s: analyze_shape(s).efficiency)
print(f' - Most efficient shape: {most_efficient_shape.get_name()}')

Copilot uses AI. Check for mistakes.

Comment on lines 64 to 69
print(f' - Efficiency: {circle_metrics.efficiency:.4f}')

print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')
print(f' - Efficiency: {rectangle_metrics.efficiency:.4f}')

Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code attempts to access an 'efficiency' attribute from ShapeMetrics, but the ShapeMetrics dataclass only has area, perimeter, name, and aspect_ratio attributes. There is no 'efficiency' property defined.

Suggested change
print(f' - Efficiency: {circle_metrics.efficiency:.4f}')
print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')
print(f' - Efficiency: {rectangle_metrics.efficiency:.4f}')
print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')

Copilot uses AI. Check for mistakes.

Comment on lines 64 to 70
print(f' - Efficiency: {circle_metrics.efficiency:.4f}')

print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')
print(f' - Efficiency: {rectangle_metrics.efficiency:.4f}')
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code attempts to access an 'efficiency' attribute from ShapeMetrics, but the ShapeMetrics dataclass only has area, perimeter, name, and aspect_ratio attributes. There is no 'efficiency' property defined.

Suggested change
print(f' - Efficiency: {circle_metrics.efficiency:.4f}')
print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')
print(f' - Efficiency: {rectangle_metrics.efficiency:.4f}')
print(f' - Efficiency: {circle_metrics.area / (circle_metrics.perimeter ** 2):.4f}')
print(f'Rectangle analysis:')
print(f' - Area: {rectangle_metrics.area:.2f}')
print(f' - Perimeter: {rectangle_metrics.perimeter:.2f}')
print(f' - Aspect ratio: {rectangle_metrics.aspect_ratio:.4f}')
print(f' - Efficiency: {rectangle_metrics.area / (rectangle_metrics.perimeter ** 2):.4f}')

Copilot uses AI. Check for mistakes.

@hakula139 hakula139 force-pushed the feat/add-python-bindings branch from 0a9bdc6 to bf01c31 Compare August 10, 2025 16:52
@hakula139 hakula139 marked this pull request as draft August 10, 2025 16:52

This comment was marked as off-topic.

@hakula139 hakula139 force-pushed the feat/add-python-bindings branch from bf01c31 to 3c60c95 Compare August 10, 2025 17:16
@hakula139 hakula139 changed the title feat: add Python bindings feat(python): add Python bindings and wrappers Aug 10, 2025
@hakula139 hakula139 force-pushed the feat/add-python-bindings branch from 3c60c95 to ebbc734 Compare August 17, 2025 12:46
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.

1 participant