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

Add log function for templates #3662

Merged
merged 1 commit into from Jul 2, 2017

Conversation

Projects
None yet
4 participants
@artem-sidorenko
Contributor

artem-sidorenko commented Jun 29, 2017

It might be very useful for building nice tag clouds

Example:

Tag cloud built without log:

untitled

Tag cloud built with log:

untitled1

@digitalcraftsman

This comment has been minimized.

Member

digitalcraftsman commented Jun 29, 2017

Go's math package names the function for the natural logarithm (log_e) just Log. You use this name for the template func as well. However, users might find this name ambiguous because I would rather expect that Log returns log_10 instead of log_e.

Hence I would prefer to name the template function ln (for logarithm naturalis).

Let's take this a step further. log_e might be limiting in some cases and log_10 might be needed instead. While the math package has a function for this too it only adds just another function to the namespace.

Hence I suggest the addition (or even as a replacement) of a more generic logarithm template func where users can set the base as a parameter. The math package doesn't provide a function that takes the base as a parameter.

But it's trivial to obtain the result in the wished by with log_10. Let's say I want to calculate log_2(8). This can be written as log_10(8) / log_10(2).

So implementing a generic function should be trivial.

tpl/math/math.go Outdated
av := reflect.ValueOf(a)
var af float64
switch av.Kind() {

This comment has been minimized.

@bep

bep Jun 29, 2017

Member

Use cast. ToFloat64.

This comment has been minimized.

@moorereason

moorereason Jun 29, 2017

Contributor

Or cast.ToFloat64E

This comment has been minimized.

@artem-sidorenko
@moorereason

This comment has been minimized.

Contributor

moorereason commented Jun 29, 2017

@digitalcraftsman,
The Go math package offers Log, Log10, Log1p, Log2, and Logb. We should follow their lead as to what Log means. I'm not in favor of adding all of these funcs to Hugo until someone needs them, though.

Also, I don't like ln. That's too short (saves one character and loses most of its intuitiveness). Makes me think of creating symlinks. If I were searching for a logarithm func, I'd be looking for "log" not "ln."

If we put these in the global funcmap, I say we simply use the same names: log, log10, log1p, log2, and logb.

Creating a single, multipurpose log func feels like over-engineering.

@bep

This comment has been minimized.

Member

bep commented Jun 29, 2017

I'd be looking for "log" not "ln."

Without any context, I would say that log does some debug logging or something. I would say that

  1. Follow the names from math.Log etc.
  2. No aliases
@digitalcraftsman

This comment has been minimized.

Member

digitalcraftsman commented Jun 29, 2017

I'm not in favor of adding all of these funcs to Hugo until someone needs them, though.

Hence the generic func which is neither difficult to use nor to implement.

Also, I don't like ln. That's too short (saves one character and loses most of its intuitiveness). Makes me think of creating symlinks. If I were searching for a logarithm func, I'd be looking for "log" not "ln."

Well, ln is the button you'll find on a calculator.

@artem-sidorenko artem-sidorenko force-pushed the artem-forks:template-log branch Jul 1, 2017

@moorereason

Please update this PR to remove the log alias/mapping and simply use the namespace method: {{ math.Log 1.0 }}.

Add log function for templates
It might be very useful for building tag clouds

@artem-sidorenko artem-sidorenko force-pushed the artem-forks:template-log branch to ee5c2f2 Jul 2, 2017

@artem-sidorenko

This comment has been minimized.

Contributor

artem-sidorenko commented Jul 2, 2017

@bep bep merged commit 34c5667 into gohugoio:master Jul 2, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@artem-sidorenko artem-sidorenko deleted the artem-forks:template-log branch Jul 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment