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

Don't render sum filter results in scientific notation. #1728

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

Conversation

jg-rp
Copy link
Contributor

@jg-rp jg-rp commented Jun 30, 2023

This pull request changes the standard sum filter to return a float instead of a BigDecimal when given one or more floats as input, as all standard math filters do.

This is necessary to prevent the result being rendered in scientific notation.

Closes #1725.

@adamklingbaum adamklingbaum self-requested a review July 19, 2023 18:44
Copy link
Contributor

@adamklingbaum adamklingbaum left a comment

Choose a reason for hiding this comment

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

Thank you for identifying and resolving this, @jg-rp. Your fix effectively makes the sum filter's behaviour consistent with other arithmetic operations in Liquid.

It would be great to see additional tests, similar to the ones you added, but with floats as property values (see example below).

Otherwise, this looks great to me.

-Adam

def test_sum_with_indexable_map_values
input = [{ "quantity" => 1 }, { "quantity" => 2, "weight" => 3 }, { "weight" => 4 }]
assert_equal(0, @filters.sum(input))
assert_equal(3, @filters.sum(input, "quantity"))
assert_equal(7, @filters.sum(input, "weight"))
assert_equal(0, @filters.sum(input, "subtotal"))
end

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

Successfully merging this pull request may close these issues.

Summing floats with the sum filter
2 participants