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.

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

Stubs for public interface #58

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Stubs for public interface #58

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
297efac
add stubs for public interface
xoudini Jul 10, 2024
b400fb6
add py.typed marker
xoudini Jul 10, 2024
3d853ef
add subscript and get type hints for output type
xoudini Jul 11, 2024
5a3cb55
use StrPath from typeshed
xoudini Aug 14, 2024
1b7f8a4
use mapping instead of dict
xoudini Aug 14, 2024
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: 35 additions & 0 deletions mjml/__init__.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import pathlib
import typing as t

if t.TYPE_CHECKING:
import typing_extensions as te
from _typeshed import SupportsRead

from mjml.core.api import Component

class _Output(t.NamedTuple):
html: str
errors: t.Sequence[str]

@te.overload
def __getitem__(self, _: t.Literal["html"]) -> str: ...
@te.overload
def __getitem__(self, _: t.Literal["errors"]) -> t.Sequence[str]: ...
@te.overload
def get(self, key: t.Literal["html"], /) -> t.Optional[str]: ...
@te.overload
def get(self, key: t.Literal["html"], default: str, /) -> str: ...
@te.overload
def get(self, key: t.Literal["errors"], /) -> t.Optional[t.Sequence[str]]: ...
@te.overload
def get(self, key: t.Literal["errors"], default: t.Sequence[str], /) -> t.Sequence[str]: ...

FpOrJson = t.Union[t.Dict[str, t.Any], str, bytes, SupportsRead[str], SupportsRead[bytes]]
Copy link
Owner

Choose a reason for hiding this comment

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

Probably bike-shedding but does mjml_to_html() works if xml_fp_or_json contains a bytes value?

Copy link
Author

@xoudini xoudini Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The BeatifulSoup constructor has the following annotation (in the stubs):

def __init__(
    self,
    markup: str | bytes | SupportsRead[str] | SupportsRead[bytes] = "",
    # ...
) -> None: ...

Purely based on that it should work, since bytes is left untouched by mjml_to_html, but I also quickly tried it, with a minimal example, and it seems to work just fine:

from mjml import mjml_to_html

mjml_to_html(b"<mjml><mj-body><mj-raw>foo</mj-raw></mj-body></mjml>")

# DotMap(html='<!doctype html>\n<html xmlns="http://www.w3.org/1999/xhtml" xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office">\n  <head>\n    <title>\n      \n    </title>\n    <!--[if !mso]><!-->\n    <meta http-equiv="X-UA-Compatible" content="IE=edge">\n    <!--<![endif]-->\n    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">\n    <meta name="viewport" content="width=device-width, initial-scale=1">\n    <style type="text/css">\n      #outlook a { padding:0; }\n      body { margin:0;padding:0;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%; }\n      table, td { border-collapse:collapse;mso-table-lspace:0pt;mso-table-rspace:0pt; }\n      img { border:0;height:auto;line-height:100%; outline:none;text-decoration:none;-ms-interpolation-mode:bicubic; }\n      p { display:block;margin:13px 0; }\n    </style>\n    <!--[if mso]>\n    <noscript>\n    <xml>\n    <o:OfficeDocumentSettings>\n      <o:AllowPNG/>\n      <o:PixelsPerInch>96</o:PixelsPerInch>\n    </o:OfficeDocumentSettings>\n    </xml>\n    </noscript>\n    <![endif]-->\n    <!--[if lte mso 11]>\n    <style type="text/css">\n      .mj-outlook-group-fix { width:100% !important; }\n    </style>\n    <![endif]-->\n    \n    \n    <style type="text/css">\n\n    \n    \n    </style>\n    <style type="text/css"></style>\n    \n  </head>\n  <body style="word-spacing:normal;">\n    \n    <div style="">foo</div>\n  </body>\n</html>', errors=[])

StrOrPath = t.Union[str, pathlib.PurePath]
xoudini marked this conversation as resolved.
Show resolved Hide resolved

def mjml_to_html(
xml_fp_or_json: "FpOrJson",
skeleton: t.Optional[str] = None,
template_dir: t.Optional["StrOrPath"] = None,
Copy link
Owner

Choose a reason for hiding this comment

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

This is one of the places where types are really helpful as I looked through the code and I am not sure if a str instance is really acceptable. It seems to me at first glance that we are doing template_dir / foo unconditionally so this should be os.PathLike.

Maybe you can check my finding? (I am probably overlooking something)

Copy link
Author

@xoudini xoudini Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It seems to me at first glance that we are doing template_dir / foo unconditionally so this should be os.PathLike.

This should be fine, as long as foo is a pathlib.PurePath (or derivative thereof), and it looks to me like this is in fact the case. I.e. pathlib.PurePath implements this operator for both left-hand and right-hand joins.

I used this annotation initially because there's already some string usage (as far as I can tell), for instance here:

mjml-python/mjml/scripts/mjml.py

Line 32 in ed3187e

result = mjml_to_html(mjml_fp, template_dir=template_dir)

custom_components: t.Optional[t.List[t.Type["Component"]]] = None,
) -> "_Output": ...
Empty file added mjml/py.typed
View file
Open in desktop
Empty file.