Skip to content
This repository was archived by the owner on Mar 1, 2024. It is now read-only.

Fix issue #27, world age problem on v0.6 #30

Merged
tkelman merged 3 commits into from
May 25, 2017
Merged

Fix issue #27, world age problem on v0.6 #30

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9151f80
Fix issue #27, world age problem on v0.6
ScottPJones Jan 28, 2017
ea6c54b
Use exact version number for when #265 fix was merged
ScottPJones Apr 25, 2017
c94496c
Removed use of at-static macro at top-level
ScottPJones Apr 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 68 additions & 3 deletions src/cformat.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,80 @@
formatters = Dict{ Compat.ASCIIString, Function }()

function sprintf1( fmt::Compat.ASCIIString, x )
@static if VERSION >= v"0.6.0-dev.1671"
Copy link
Copy Markdown
Contributor

@tkelman tkelman Apr 25, 2017

Choose a reason for hiding this comment

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

is static absolutely needed here?

Copy link
Copy Markdown
Collaborator Author

@ScottPJones ScottPJones Apr 25, 2017

Choose a reason for hiding this comment

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

I'd believed it was the preferred way to deal with sections of code that should not be compiled because of platform or version differences.
Is that not the case?

Copy link
Copy Markdown
Contributor

@tkelman tkelman Apr 25, 2017

Choose a reason for hiding this comment

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

It's completely unnecessary at top-level

Copy link
Copy Markdown
Collaborator Author

@ScottPJones ScottPJones Apr 30, 2017

Choose a reason for hiding this comment

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

OK, that's good to know (it might be nice to have the docstring for @static mention that)
In my own code, I'd just replaced any of the old macros (@windows etc. with @static if ... everwhere, and didn't realize that the @static part was not needed at the top level (does it hurt anything though to leave it in?)

Copy link
Copy Markdown
Contributor

@tkelman tkelman Apr 30, 2017

Choose a reason for hiding this comment

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

it means code in the not evaluated conditional won't trigger warnings or some errors, which sometimes is useful but other times is like ifdefs in c, can hide issues if you only ever develop on systems where one of the branches is true and never the other

Copy link
Copy Markdown
Collaborator Author

@ScottPJones ScottPJones Apr 30, 2017

Choose a reason for hiding this comment

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

OK, that makes sense, I've removed the @static

Copy link
Copy Markdown
Collaborator Author

@ScottPJones ScottPJones Apr 30, 2017

Choose a reason for hiding this comment

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

Although, in these cases where I've used it to distinguish between v0.5.x and v0.6, both branches are being tested and used (with Travis-CI on GitHub, and at work, where even though we are deploying now on v0.5.1, I'm making sure that we'll be ready for v0.6 as soon as it is released and all of the packages that we use have been updated to not have any deprecations)


sprintf1( fmt::Compat.ASCIIString, x ) = eval(Expr(:call, generate_formatter( fmt ), x))

function checkfmt(fmt)
test = Base.Printf.parse( fmt )
(length( test ) == 1 && typeof( test[1] ) <: Tuple) ||
error( "Only one AND undecorated format string is allowed")
end

function generate_formatter( fmt::Compat.ASCIIString )
global formatters
f = generate_formatter( fmt )
f( x )

haskey( formatters, fmt ) && return formatters[fmt]

if !contains( fmt, "'" )
checkfmt(fmt)
return (formatters[ fmt ] = @eval(x->@sprintf( $fmt, x )))
end

conversion = fmt[end]
conversion in "sduifF" ||
error( string("thousand separator not defined for ", conversion, " conversion") )

fmtactual = replace( fmt, "'", "", 1 )
checkfmt( fmtactual )
conversion in "sfF" ||
return (formatters[ fmt ] = @eval(x->checkcommas(@sprintf( $fmtactual, x ))))

formatters[ fmt ] =
if endswith( fmtactual, 's')
@eval((x::Real)->((eltype(x) <: Rational)
? addcommasrat(@sprintf( $fmtactual, x ))
: addcommasreal(@sprintf( $fmtactual, x ))))
else
@eval((x::Real)->addcommasreal(@sprintf( $fmtactual, x )))
end
end

function addcommasreal(s)
dpos = findfirst( s, '.' )
dpos != 0 && return string(addcommas( s[1:dpos-1] ), s[ dpos:end ])
# find the rightmost digit
for i in length( s ):-1:1
isdigit( s[i] ) && return string(addcommas( s[1:i] ), s[i+1:end])
end
s
end

function addcommasrat(s)
# commas are added to only the numerator
spos = findfirst( s, '/' )
string(addcommas( s[1:spos-1] ), s[spos:end])
end

function checkcommas(s)
for i in length( s ):-1:1
if isdigit( s[i] )
s = string(addcommas( s[1:i] ), s[i+1:end])
break
end
end
s
end

else

sprintf1( fmt::Compat.ASCIIString, x ) = (generate_formatter( fmt ))(x)

function generate_formatter( fmt::Compat.ASCIIString )
global formatters
if haskey( formatters, fmt )
return formatters[fmt]
end

func = @compat Symbol("sprintf_", replace(base64encode(fmt), "=", "!"))

if !contains( fmt, "'" )
Expand Down Expand Up @@ -77,6 +141,7 @@ function generate_formatter( fmt::Compat.ASCIIString )
formatters[ fmt ] = f
f
end
end

function addcommas( s::Compat.ASCIIString )
len = length(s)
Expand Down
22 changes: 12 additions & 10 deletions test/cformat.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ _erfinv(z) = sqrt(π) * Base.Math.@horner(z, 0, 1, 0, π/12, 0, 7π^2/480, 0, 12
function test_equality()
println( "test cformat equality...")
srand(10)
fmts = Compat.ASCIIString[ "%10.4f", "%f", "%e", "%10f", "%.3f", "%.3e" ]
for fmt in fmts
l = :( x-> x )
l.args[2].args[2] = @compat Expr(:macrocall, Symbol("@sprintf"), fmt, :x)
mfmtr = eval( l )
fmts = [ (x->@sprintf("%10.4f",x), "%10.4f"),
(x->@sprintf("%f", x), "%f"),
(x->@sprintf("%e", x), "%e"),
(x->@sprintf("%10f", x), "%10f"),
(x->@sprintf("%.3f", x), "%.3f"),
(x->@sprintf("%.3e", x), "%.3e")]
for (mfmtr,fmt) in fmts
for i in 1:10000
n = _erfinv( rand() * 1.99 - 1.99/2.0 )
expect = mfmtr( n )
Expand All @@ -21,11 +23,11 @@ function test_equality()
end
end

fmts = Compat.ASCIIString[ "%d", "%10d", "%010d", "%-10d" ]
for fmt in fmts
l = :( x-> x )
l.args[2].args[2] = @compat Expr(:macrocall, Symbol("@sprintf"), fmt, :x)
mfmtr = eval( l )
fmts = [ (x->@sprintf("%d",x), "%d"),
(x->@sprintf("%10d",x), "%10d"),
(x->@sprintf("%010d",x), "%010d"),
(x->@sprintf("%-10d",x), "%-10d")]
for (mfmtr,fmt) in fmts
for i in 1:10000
j = round(Int, _erfinv( rand() * 1.99 - 1.99/2.0 ) * 100000 )
expect = mfmtr( j )
Expand Down