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

WIP: string formatting Julep #49

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

WIP: string formatting Julep #49

wants to merge 12 commits into from

Conversation

simonbyrne
Copy link
Contributor

No description provided.

@simonbyrne
Copy link
Contributor Author

Thanks to @andyferris for the idea.

Formatting.md Outdated

where `stringformat` is the appropriately defined function returning a formatted string.

However this may result in large numbers of intermediate allocations of small strings. Ideally we would want this to lower to something like
Copy link
Member

@stevengj stevengj Feb 5, 2018

Choose a reason for hiding this comment

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

Interpolation could instead call something like stringformat(io, pi, fracdigits=4) on the io::IOBuffer that is used for string construction by string.

Maybe "π is approximately $(pi, fracdigits=4)." could instead lower string("π is approximately ", StringFormat(pi, fracdigits=4), "."), where a f::StringFormat{T,KW} object caches both the object pi::T and a namedtuple (fracdigits=4,)::KW, and has a print(io, f) method (used by both string and print).

Copy link
Member

Choose a reason for hiding this comment

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

i.e.

immutable StringFormat{T,KW}
    x::T # the value to be printed
    kws::KW # NamedTuple of keyword arguments
end
write(io::IO, f::StringFormat) = write(io, f.x) # fallback method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this wouldn't immediately help with the print(io, "π is approximately $(pi, fracdigits=4).") case though.

Copy link
Member

Choose a reason for hiding this comment

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

You would pass StringFormat(...) in that case.

I don’t think we’d want $ there

Copy link
Member

Choose a reason for hiding this comment

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

Wait, I misread it. Yes, you’d write twice there. Not sure it’s a big deal.

Copy link
Member

Choose a reason for hiding this comment

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

One option would be to lower to LazyString(...) instead of string(...)

@andyferris
Copy link
Member

I was wondering - do we need a new stringformat function or can we just add keyword arguments to our string methods? E.g. JuliaLang/julia#25804

@simonbyrne
Copy link
Contributor Author

We could for keyword arguments, but not for positional (since string(a,b) is equivalent to string(a)*string(b)).

@stevengj
Copy link
Member

stevengj commented Feb 6, 2018

Just to pull together my (now hidden) suggestions from above, what if we made "π is approximately $(pi, fracdigits=4)." lower to something like:

LazyString(("π is approximately ", StringFormat(pi, fracdigits=4), "."))

with the definitions:

# String-like object formed by lazily concatenating printed args
struct LazyString{T<:Tuple} <: AbstractString
    args::T
end
function write(io::IO, ls::LazyString)
    # write this loop more cleverly to unroll at compile-time
    for arg in ls.args; print(buf, arg); end 
end
String(ls::LazyString) = let buf=IOBuffer(); write(buf, ls); String(take!(buf)); end
# ... other string methods...

struct StringFormat{T,KW}
    x::T # the value to be printed
    kws::KW # NamedTuple of keyword arguments
end
write(io::IO, f::StringFormat) = print(io, f.x) # fallback method

Then you could write specialized StringFormat methods as desired to handle various keyword options for specific types T.

Using a LazyString object would allow common things like print(io, "foo $bar") to be fast, allocating minimal temporary storage and making no string copies. On the other hand, other operations on a LazyString object would potentially be slow, especially if it is stored and re-used … in such cases, you should convert to a String. The construction of a LazyString might also be surprising to new users.

(This would be a breaking change since "foo $bar" would also no longer produce String.)

@c42f
Copy link
Member

c42f commented Feb 6, 2018

I can see why we'd want LazyString in principle, but to have a = "foo $bar" not produce a string would be pretty surprising for casual use. That kind of seems like a bad user experience to me and a bit overly complex without a really compelling benchmark. On the whole eager formatting seems like a better default to me. It seems reasonable that the relatively small amount of performance critical string printing code can pay the syntax burden of using Simon's @print macro or something similar.

@simonbyrne
Copy link
Contributor Author

I agree that from a usability perspective, strings should be the default result. Another option would be to make the lazy version available via a string macro?

@KristofferC
Copy link
Member

KristofferC commented Feb 13, 2018

A negative thing about having the formatting inline is that it makes it harder to read the string:

"π is approximately $(e, fracdigits=6, someother=2), while e is approximately $(e, fracdigits=6, someother=2) ."

vs

"π is approximately ${pi}, while e is approximately ${e}.".format(
    pi = fmt(pi, fracdigits = 6, someother = 6,
     e = fmt(e,  fracdigits = 6, someother = 6)

Separating content and formatting is usually also a good idea.

@simonbyrne
Copy link
Contributor Author

@KristofferC I haven't seen any other language do that sort of thing, but we could do that via let blocks

let  pi = fmt(pi, fracdigits = 6, someother = 6), e = fmt(e,  fracdigits = 6, someother = 6)
    "π is approximately $pi, while e is approximately $e."
end

@stevengj
Copy link
Member

Note that all of this could potentially be implemented in a package if you are willing to use a string macro, e.g. f"...".

@MikeInnes
Copy link
Member

You could just do "π is approximately $(fracdigits(π, 4)).", where fracdigits can return a stringformat object. Seems like this is more easily extensible in that I can define my own rounding routine, for example.

@stevengj
Copy link
Member

@MikeInnes, the problem with that is that it forces the construction of a temporary string, which slows down I/O.

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.

6 participants