Skip to content
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

feat: visualize list of FSWs #11

Merged
merged 11 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
26 changes: 0 additions & 26 deletions .github/workflows/lint.yaml

This file was deleted.

31 changes: 31 additions & 0 deletions .github/workflows/lint_and_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Lint and Test
Copy link
Contributor

Choose a reason for hiding this comment

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

Why lint and test? when separate, they give two different information types, and when together it will first fail on lint, then test and not communicate

Copy link
Contributor Author

@bipinkrish bipinkrish Oct 18, 2024

Choose a reason for hiding this comment

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

I did not get you? do you not want check lint and test in same run? because it doesn't stop if one fails?

Copy link
Contributor

Choose a reason for hiding this comment

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

i prefer checking them in separate runs because i would like to know the status of both.

  • if test fails, i really need to fix something
  • if lint fails, it can wait for the next commit. i should fix it but less rush
    so having the two workflows gives me two status indicators

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


on:
push:
branches: [master, main]
pull_request:
branches: [master, main]

jobs:
lint_and_test:
name: Lint and Test
runs-on: ubuntu-latest

strategy:
matrix:
python-version: ["3.8"] # "3.10", "3.12" can be added when we figure out to add pylint: diable=too-many-positional-arguments to 3.8

steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install Requirements
run: pip install .[dev,mouthing,server]

- name: Lint Code
run: pylint signwriting

- name: Test Code
run: pytest signwriting
26 changes: 0 additions & 26 deletions .github/workflows/test.yaml

This file was deleted.

5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ mouthing = [
"epitran",
"g2pk"
]
server = [
# For API server
"flask",
"Flask-RESTful",
bipinkrish marked this conversation as resolved.
Show resolved Hide resolved
]

[tool.yapf]
based_on_style = "google"
Expand Down
2 changes: 1 addition & 1 deletion signwriting/tokenizer/base_tokenizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

class BaseTokenizer:

# pylint: disable=too-many-arguments,too-many-instance-attributes
# pylint: disable=too-many-arguments, too-many-instance-attributes
def __init__(self,
tokens: List[str],
starting_index=None,
Expand Down
3 changes: 2 additions & 1 deletion signwriting/utils/join_signs.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from collections import namedtuple
from typing import List

from signwriting.formats.fsw_to_sign import fsw_to_sign
from signwriting.formats.sign_to_fsw import sign_to_fsw
Expand Down Expand Up @@ -59,7 +60,7 @@ def join_signs_horizontal(*fsws: str, spacing: int = 0):
Point = namedtuple("Point", ["x", "y"])


def sign_from_symbols(symbols: list[SignSymbol], fix_x=True, fix_y=True) -> Sign:
def sign_from_symbols(symbols: List[SignSymbol], fix_x=True, fix_y=True) -> Sign:
min_p = Point(x=999, y=999)
max_p = Point(x=0, y=0)
for symbol in symbols:
Expand Down
Binary file added signwriting/visualizer/test_assets/horizontal.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added signwriting/visualizer/test_assets/vertical.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 10 additions & 1 deletion signwriting/visualizer/test_visualize.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from PIL import Image
from numpy.testing import assert_array_equal

from signwriting.visualizer.visualize import signwriting_to_image
from signwriting.visualizer.visualize import signwriting_to_image, signwritings_to_image


class VisualizeCase(unittest.TestCase):
Expand Down Expand Up @@ -76,6 +76,15 @@ def test_image_with_fill_and_embedded_color(self):
fill_color=(123,234,0,255))
self.assert_image_equal_with_reference(fsw, image)

def test_signwritings_to_image(self):
fsw1 = "AS10011S10019S2e704S2e748M525x535S2e748483x510S10011501x466S20544510x500S10019476x475"
fsw2 = "M530x518S19a30500x482S19a38465x481S22f04509x506S22f14467x504"

image = signwritings_to_image([fsw1, fsw2], direction="horizontal")
self.assert_image_equal_with_reference("horizontal", image)

image = signwritings_to_image([fsw1, fsw2], direction="vertical")
self.assert_image_equal_with_reference("vertical", image)

if __name__ == '__main__':
unittest.main()
36 changes: 35 additions & 1 deletion signwriting/visualizer/visualize.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# pylint: disable=unnecessary-lambda-assignment
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally, this should be near the affected line, otherwise in the pyproject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there were 4 lines using lambda i felt it is redundant to use it 4 time, so what do you suggest i do? in the pyproject config?

Copy link
Contributor

Choose a reason for hiding this comment

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

either project config or next to each line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


from functools import lru_cache
from pathlib import Path
from typing import Tuple
from typing import Tuple, List, Literal

from PIL import Image, ImageDraw, ImageFont

Expand Down Expand Up @@ -65,3 +67,35 @@ def signwriting_to_image(fsw: str, antialiasing=True, trust_box=True, embedded_c
font=line_font, embedded_color=embedded_color)

return img


# pylint: disable=too-many-locals, too-many-arguments
def signwritings_to_image(fsw_list: List[str], antialiasing: bool = True, trust_box: bool = True,
Copy link
Contributor

Choose a reason for hiding this comment

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

"signwritings" is very not elegant imo.
i think this can use the same function as the original, but when encountering a list, call layout

def signwriting_to_image(fsw: Union[str, List[str]], antialiasing=True, trust_box=True, embedded_color=False,
                         line_color: RGBA = (0, 0, 0, 255),
                         fill_color: RGBA = (255, 255, 255, 255), 
                         direction: Literal["horizontal", "vertical"] = "horizontal") -> Image:
    if isinstance(fsw, list):
        images = [
            signwriting_to_image(fsw_string, antialiasing, trust_box, embedded_color, line_color, fill_color)
            for fsw_string in fsw_list
        ]
        return layout_signwriting(images, direction)
   
    .....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

embedded_color: bool = False, line_color: Tuple[int, int, int, int] = (0, 0, 0, 255),
fill_color: Tuple[int, int, int, int] = (255, 255, 255, 255),
direction: Literal["horizontal", "vertical"] = "horizontal") -> Image:
images = [
signwriting_to_image(fsw_string, antialiasing, trust_box, embedded_color, line_color, fill_color)
for fsw_string in fsw_list
]

if direction == "horizontal":
max_height = max(img.height for img in images)
total_width = sum(img.width for img in images)
size = (total_width, max_height)
paste_position = lambda offset: (offset, 0)
offset_increment = lambda img: img.width
else:
max_width = max(img.width for img in images)
total_height = sum(img.height for img in images)
size = (max_width, total_height)
paste_position = lambda offset: (0, offset)
offset_increment = lambda img: img.height

final_image = Image.new("RGBA", size, (255, 255, 255, 0))
offset = 0
for img in images:
final_image.paste(img, paste_position(offset))
offset += offset_increment(img)

return final_image
Copy link
Contributor

@AmitMY AmitMY Oct 18, 2024

Choose a reason for hiding this comment

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

I do not like the output of this layout function.
See here for example https://slevinski.github.io/SuttonSignWriting/characters/index.html, Hello World example or Introduction.
image

When multiple signs, there are rules on how to lay them out.
I believe this is the code used for a vertical column: https://github.com/sutton-signwriting/sgnw-components/tree/master/src/components/fsw-vp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean to say i should add gaps in between?

Copy link
Contributor

Choose a reason for hiding this comment

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

and center the images in the "lane"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Loading