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

Type checking strictness #40

Closed
pandvan opened this issue Oct 13, 2021 · 4 comments
Closed

Type checking strictness #40

pandvan opened this issue Oct 13, 2021 · 4 comments

Comments

@pandvan
Copy link

pandvan commented Oct 13, 2021

I'm trying to render the following template with liquidpy.

{{ 'now' | date: '%s' | times: 1000 | minus: 1800000 }}

It should be valid Liquid, it's properly rendered using various online tester tools.
Using liquidpy leads to a TypeError

TypeError: unsupported operand type(s) for *: 'DateTime' and 'int'

as date are managed via DateTime class that only provide plus and minus operations at the moment.
I don't know how Liquid handles data types. Is there any way I can force/cast to a specific data type (int in this case), "bypassing" Python type checking and so make it works?

p.s. I hit this liquidpy issue as I'm a user of Shuffle automation platform.

@pwwang
Copy link
Owner

pwwang commented Oct 13, 2021

Related: #38

There is a difference in type handling between ruby liquid and liquidpy:

You can do this in ruby liquid:

{{ "1" | plus: 1 }}  # 2

However, this is not valid in liquidpy. Because the template is eventually compiled into python code and the type handling is delegated to python, but "1" + 1 is not a valid python operation.

To solve this, a Datetime class was introduced, which renders the date into string by given format, while handling plus and minus by regarding the object as a python datetime object (#38). However (@frikky), there is a misunderstanding about the issue. After the arithmetic operation, the object should be turned into numbers, instead of a Datetime object.

This should still be a valid template to render:

{{ "now" | date: "%s" | plus: 86400 | date: "%Y-%m-%d" }}

But now it is not.

So, the plus, minus and other arithmetic operations managed by Datetime will return numbers in the next version to make the above template working.

However, {{ "now" | date: "%s" }} should hold a string instead of a number, literally, just like the very first simple template I showed. Putting it in an arithmetic operation directly confuses people since that simple one doesn't work.

A universal way to do it is to convert the string into a number explicitly:

{{ "1" | int | plus: 1}}  # 2
{{ 'now' | date: '%s' | int | times: 1000 | minus: 1800000 }}
# 1634141733000

Now, because the typecasting is handled externally (by the filter int), other than internally, by the Datetime class, so that Datetime class is not necessary to exist anymore.

It will still be available to do an arithmetic operation in the next minor release (0.7.2) with the output from date:

{{ "now" | date: "%s" | plus: 86400 }}  # 1634230267

However, we should direct people to handle the typecasting themselves to avoid confusion.

pwwang added a commit that referenced this issue Oct 13, 2021
@pwwang pwwang mentioned this issue Oct 13, 2021
@pandvan
Copy link
Author

pandvan commented Oct 13, 2021

You can do this in ruby liquid:

{{ "1" | plus: 1 }}  # 2

However, this is not valid in liquidpy. Because the template is eventually compiled into python code and the type handling is delegated to python, but "1" + 1 is not a valid python operation.

Just hit a similar issue. Capture tag results in a string and so it's not possible to do some arithmetic operation after.
A little example:

{% capture lst_size %}4{% endcapture %}
{{ 2 | at_most: lst_size }}

A universal way to do it is to convert the string into a number explicitly:

{{ "1" | int | plus: 1}}  # 2

However, we should direct people to handle the typecasting themselves to avoid confusion.

I'm totally agree that explicit typecasting specified by the user could be the best solution.
Do you think is viable to add those typecasting filters in liquidpy?

@pwwang
Copy link
Owner

pwwang commented Oct 13, 2021

Yes, at the next release (0.7.2), int, float, str and bool are added as filters and globals. So you should be able to do this:

{% capture lst_size | int %}4{% endcapture %}
{{ 2 | at_most: lst_size }}

or use int as a global:

{% capture lst_size %}4{% endcapture %}
{{ 2 | at_most: int(lst_size) }}     

See also the docs just added: #41 (comment)

pwwang added a commit that referenced this issue Oct 19, 2021
* 🐛 Fix date filter issues (#38, #40)

* ✨ Add markdownify for jekyll

* 🔖 0.7.2

* 💚 Add markdown dep in CI

* ✅ Fix date filter tests

* ✨ Add int, float, str and bool as both filters and globals for all modes

* 📝 Update docs for builtin typecasting filters/globals.

* 📝 Update docs

* ✨ Add jekyll filter `number_of_words`

* ✨ Add jekyll filter `sort`

* ✨ Add jekyll filter `slugify`

* ✨ Add jekyll filter `array_to_sentence_string`

* ✨ Add jekyll filter `jsonify`

* ✨ Add jekyll filters `xml_escape`, `cgi_escape` and `uri_escape`
@pwwang
Copy link
Owner

pwwang commented Oct 19, 2021

0.7.2 is released.

@pwwang pwwang closed this as completed Oct 19, 2021
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

No branches or pull requests

2 participants