to_number filter not working correctly

[RadValentin]

Using this example from the documentation produces wrong results. The message will always be printed regardless of the value of num

{% capture num %}8000{% endcapture %}
{% if num | to_number > 9000 %}
{{ num }} is over 9000!
{% endif %}

[fw42]

@jamesmacaulay, do you remember why the to_number filter is private? (there is a test for it written by you that asserts it)

[fw42]

If it's always been private, it probably shouldn't show up in the documentation. But then, it seems useful, so I'm wondering why we don't make it public.

[fw42]

cc @dtKinger

[jamesmacaulay]

I think that originally, @tobi didn't want there to be any explicit type casting in liquid. to_number was just a helper to get the arithmetic filters working correctly:

fce8bcb

It looks like @Tetsuro did the to_number docs for Shopify. @Tetsuro, was to_number exposed as a filter in Shopify?

[tobi]

I'm surprised it produces the wrong result. It's supposed to work ( like in php, smarty ). to_number should definitely not be exposed because Liquid has no datatypes. That's a programming concept with shouldn't be leaked into a template language.

[fw42]

It doesn't work because the method is private. There is even a test that asserts that it's broken in this way. Shouldn't it rather raise a Liquid error instead of swallowing it?

[fw42]

@Tetsuro, @dtKinger, can you please remove this filter from the Shopify documentation? It doesn't work and also will not work in the future.

[dtKinger]

No problem, I'll omit it asap

[jamesmacaulay]

Okay another important point: filters are not supported in if statements. If you need to use the result of a filter in a conditional, you need to put it in a variable with assign or capture first (assign does now allow conditionals).

It looks like invalid filters like to_number or i_am_not_a_filter are all just treated as no-ops (they return their input unchanged).

Right now trying to do {% if string > 9000 %} gives you "Liquid error: comparison of String with 9000 failed". Given Liquid's philosophy of no explicit types, presumably we should make anything that expects a particular type to coerce its input to that type behind the scenes (e.g. > should coerce everything to a number).

[tobi]

Agreed. I'm puzzled while this isn't the case right now.

On Monday, March 24, 2014, James MacAulay notifications@github.com wrote:

Okay another important point: filters are not supported in if statements.
If you need to use the result of a filter in a conditional, you need to put
it in a variable with assign or capture first (assign does now allow
conditionals).

It looks like invalid filters like to_number or i_am_not_a_filter are all
just treated as no-ops (they return their input unchanged).

Right now trying to do {% if string > 9000 %} gives you "Liquid error:
comparison of String with 9000 failed". Given Liquid's philosophy of no
explicit types, presumably we should make anything that expects a
particular type to coerce its input to that type behind the scenes (e.g. >should coerce everything to a number).

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/326#issuecomment-38491109
.

• tobi
  CEO Shopify

[RadValentin]

@jamesmacaulay please remember that relational operators (<, >, etc.) can also apply to strings in which case it turns into lexicographical comparison, 'a' < 'b' and 'ab' > 'aa'. to_number might be useful in such cases where you explicitly want a string to be treated a number.

[jamesmacaulay]

Yeah, that's how it is in ruby. However I think in Liquid, if the philosophy really is "variables don't have types", the thinking would be that there should be distinct operators which only do lexicographic comparison (and cast all arguments as strings).