Skip to content

feat: Support all stroke versions #6

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.

Sign up for GitHub

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

Open
dmoreno-at-wiris wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Support all stroke versions #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions py_path_signature/data_models/stroke.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
from abc import abstractmethod
from typing import List, Tuple, Annotated, Iterable
import math

from py_path_signature.data_models.basic import BasicModel
from py_path_signature.data_models.error_messages import ERROR_MESSAGES
from pydantic import root_validator, validator
from pydantic import root_validator, validator, Field
from pydantic.types import conlist

class Stroke(BasicModel):

@abstractmethod
def x_generator(self) -> Iterable[float]:
pass

@abstractmethod
def y_generator(self) -> Iterable[float]:
pass

class StrokeFormatError(Exception):
"""Custom error that is raised when an input stroke doesn't have the right format."""
Expand All @@ -14,7 +25,7 @@ def __init__(self, message: str) -> None:
super().__init__(message)


class Stroke(BasicModel):
class StrokeDict(Stroke):

x: conlist(float, min_items=1)
y: conlist(float, min_items=1)
Expand All @@ -35,3 +46,23 @@ def check_value_is_nan(cls, value):
if math.isnan(value):
raise StrokeFormatError(message=ERROR_MESSAGES["NAN_DETECTED"])
return value

def x_generator(self) -> Iterable[float]:
for i in self.x:
yield i

def y_generator(self) -> Iterable[float]:
for i in self.y:
yield i

Comment on lines +51 to +57
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The generator simply iterates and yields items from self.x without transformation. This can be simplified to yield from self.x for better performance and cleaner code. The same applies to y_generator() on lines 54-56.

Suggested change
for i in self.x:
yield i
def y_generator(self) -> Iterable[float]:
for i in self.y:
yield i
yield from self.x
def y_generator(self) -> Iterable[float]:
yield from self.y

Copilot uses AI. Check for mistakes.
class StrokeTuple(Stroke):

coordinates: Annotated[List[Tuple[float, float]], Field(min_length=2)]
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The minimum length is set to 2, but StrokeDict requires min_items=1 for x and y lists. This inconsistency means StrokeTuple requires at least 2 coordinate pairs while StrokeDict can have just 1 point. Consider using min_length=1 for consistency across stroke formats.

Suggested change
coordinates: Annotated[List[Tuple[float, float]], Field(min_length=2)]
coordinates: Annotated[List[Tuple[float, float]], Field(min_length=1)]

Copilot uses AI. Check for mistakes.

def x_generator(self) -> Iterable[float]:
for (x,y) in self.coordinates:
yield x

def y_generator(self) -> Iterable[float]:
for (x,y) in self.coordinates:
Comment on lines +63 to +67
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Missing space after comma in tuple unpacking. Should be for (x, y) in self.coordinates: to follow PEP 8 style guidelines. Same issue on line 67.

Suggested change
for (x,y) in self.coordinates:
yield x
def y_generator(self) -> Iterable[float]:
for (x,y) in self.coordinates:
for (x, y) in self.coordinates:
yield x
def y_generator(self) -> Iterable[float]:
for (x, y) in self.coordinates:

Copilot uses AI. Check for mistakes.
yield y
Comment on lines +62 to +68
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Both generators iterate over self.coordinates separately, which could be inefficient for large coordinate lists accessed sequentially. Consider if the calling code in path_signature_extractor.py could be refactored to iterate once and unpack coordinates, avoiding duplicate iteration.

Copilot uses AI. Check for mistakes.
6 changes: 3 additions & 3 deletions py_path_signature/path_signature_extractor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ def calculate_bounding_box(strokes: List[Stroke]) -> Tuple[int, int, int, int]:
if len(strokes) == 0:
raise Exception("Empty list of strokes.")

x_values = [x for stroke in strokes for x in stroke.x]
y_values = [y for stroke in strokes for y in stroke.y]
x_values = [x for stroke in strokes for x in stroke.x_generator()]
y_values = [y for stroke in strokes for y in stroke.y_generator()]

x_min = min(x_values)
x_max = max(x_values)
Expand Down Expand Up @@ -140,7 +140,7 @@ def from_coordinates_to_pixels(self, strokes: List[Stroke]) -> List[List[Tuple[i
pixels = []
for stroke in strokes:
out_stroke = []
for (x, y) in zip(stroke.x, stroke.y):
for (x, y) in zip(stroke.x_generator(), stroke.y_generator()):
x = (self.rendering_size[1] - 1) * (x - x_min) / w
y = (self.rendering_size[0] - 1) * (y - y_min) / h

Expand Down
Loading