Stubs for public interface #58
Conversation
|
See mypy docs on stubs for more details. The stubs can be deleted later, when the package is typed internally. |
|
Also added type hints for the dictionary interface of the output (since |
mjml/__init__.pyi
Outdated
| def mjml_to_html( | ||
| xml_fp_or_json: "FpOrJson", | ||
| skeleton: t.Optional[str] = None, | ||
| template_dir: t.Optional["StrOrPath"] = None, |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
It seems to me at first glance that we are doing
template_dir / foounconditionally so this should beos.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
mjml/__init__.pyi
Outdated
| @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]] |
There was a problem hiding this comment.
Probably bike-shedding but does mjml_to_html() works if xml_fp_or_json contains a bytes value?
There was a problem hiding this comment.
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=[])
No worries at all! Speaking of bike-shedding, there's currently an annotation for Lines 22 to 23 in ed3187e Since this list isn't actually mutated, it would be sufficient to annotate it as a |
This is a minimal PR, which (mostly) resolves #50, as discussed in #57.
For overriding components in downstream projects, the public interface of all components should be typed accordingly, but I'll leave that for the next PR (unless it's seen as pertinent to do here while we're at it).
For instance, calls like this:
mjml-python/tests/custom_components_test.py
Lines 28 to 31 in ed3187e
...will cause mypy to error with:
The same goes for e.g.
default_attrsand so on, so type annotations should be added to these as soon as possible.