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.

Sign up for GitHub

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

Hh/general numbers #52

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

Hh/general numbers #52

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d2dafff
Allow for Complex and Rationals and user types
hhaensel Dec 17, 2019
3c88392
type conversion for formatting methods
hhaensel Dec 17, 2019
ed836bd
printfmt returned true if suffix was empty
hhaensel Dec 17, 2019
4a958ee
correction in fmtspec and fmt test
hhaensel Dec 17, 2019
7d43657
Define constants for complex subtypes
hhaensel Dec 17, 2019
874d042
add Rationals and some code beautifying
hhaensel Dec 17, 2019
cf67f99
remove type instability
hhaensel Dec 18, 2019
4479a8a
new try catch in printfmt
hhaensel Dec 18, 2019
d07f241
adapt to master updates
hhaensel Dec 18, 2019
314a4fb
change version to 1.2.0
hhaensel Dec 18, 2019
38e386a
apply changes to `format()`
hhaensel Dec 19, 2019
82b9b43
Update tests
ScottPJones Dec 17, 2019
befc01c
apply changes to `cfmt()` and add tests
hhaensel Dec 20, 2019
ab0bdac
resolve merge conflict
hhaensel Dec 20, 2019
7598556
separator handling for general Number types
hhaensel Dec 20, 2019
0233e95
add cformat tests for complex numbers
hhaensel Dec 20, 2019
9e1d553
correct complex types and add tests
hhaensel Dec 20, 2019
0eb2a8c
include `Char` in the types of `cfmt` in order to match calls like `c…
hhaensel Jan 21, 2020
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
21 changes: 17 additions & 4 deletions src/fmt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function DefaultSpec(c::AbstractChar, syms...; kwargs...)
end
end

const DEFAULT_FORMATTERS = Dict{DataType, DefaultSpec}()
const DEFAULT_FORMATTERS = Dict{Type{<:Any}, DefaultSpec}()

# adds a new default formatter for this type
default_spec!(::Type{T}, c::AbstractChar) where {T} =
Expand Down Expand Up @@ -73,12 +73,21 @@ end
# methods to get the current default objects
# note: if you want to set a default for an abstract type (i.e. AbstractFloat)
# you'll need to extend this method like here:

const ComplexInteger = Complex{<:Integer}
const ComplexFloat = Complex{<:AbstractFloat}
const ComplexRational = Complex{<:Rational}

default_spec(::Type{<:Integer}) = DEFAULT_FORMATTERS[Integer]
default_spec(::Type{<:AbstractFloat}) = DEFAULT_FORMATTERS[AbstractFloat]
default_spec(::Type{<:AbstractString}) = DEFAULT_FORMATTERS[AbstractString]
default_spec(::Type{<:AbstractChar}) = DEFAULT_FORMATTERS[AbstractChar]
default_spec(::Type{<:AbstractIrrational}) = DEFAULT_FORMATTERS[AbstractIrrational]
default_spec(::Type{<:Rational}) = DEFAULT_FORMATTERS[Rational]
default_spec(::Type{<:Number}) = DEFAULT_FORMATTERS[Number]
default_spec(::ComplexInteger) = DEFAULT_FORMATTERS[ComplexInteger]
Copy link
Member

Choose a reason for hiding this comment

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

Again, you are confusing passing an instance of a particular type, with passing the type itself.

You need to have ::Type{<:Complex}.

I think it's best to work on making any complex type work at first, before trying to add the capability of having separate defaults for different parameterized Complex types.

default_spec(::ComplexFloat) = DEFAULT_FORMATTERS[ComplexFloat]
default_spec(::ComplexRational) = DEFAULT_FORMATTERS[ComplexRational]

default_spec(::Type{T}) where {T} =
get(DEFAULT_FORMATTERS, T) do
Expand Down Expand Up @@ -189,7 +198,7 @@ function fmt end
# TODO: do more caching to optimize repeated calls

# creates a new FormatSpec by overriding the defaults and passes it to pyfmt
# note: adding kwargs is only appropriate for one-off formatting.
# note: adding kwargs is only appropriate for one-off formatting.
# normally it will be much faster to change the fmt_default formatting as needed
function fmt(x; kwargs...)
fspec = fmt_default(x)
Expand Down Expand Up @@ -220,9 +229,13 @@ end
for (t, c) in [(Integer,'d'),
(AbstractFloat,'f'),
(AbstractChar,'c'),
(AbstractString,'s')]
(AbstractString,'s'),
(ComplexInteger,'d'),
Copy link
Member

Choose a reason for hiding this comment

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

For now, please remove the separate ComplexInteger, ComplexFloat, ComplexRational default specs.

(ComplexFloat,'f')]
default_spec!(t, c)
end

default_spec!(Number, 's', :right)
default_spec!(Rational, 's', :right)
default_spec!(AbstractIrrational, 's', :right)
default_spec!(ComplexRational, 's', :right)
default_spec!(Number, 's', :right)
40 changes: 40 additions & 0 deletions src/fmtcore.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# core formatting functions
export fmt_Number

### auxiliary functions

Expand Down Expand Up @@ -262,3 +263,42 @@ function _pfmt_specialf(out::IO, fs::FormatSpec, x::AbstractFloat)
end
end

function _pfmt_Number_f(out::IO, fs::FormatSpec, x::Number, _pf::Function)
Copy link
Member

Choose a reason for hiding this comment

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

These still have the issues that I mentioned before.

fsi = FormatSpec(fs, width = -1)
f = x->begin
fx = AbstractFloat(x) # not float(x), this should error out, if conversion is not possible
io = IOBuffer()
_pf(io, fsi, fx)
String(take!(io))
end
s = fmt_Number(x, f)
_pfmt_s(out, fs, s)
end

function _pfmt_Number_i(out::IO, fs::FormatSpec, x::Number, op::Op, _pf::Function) where {Op}
fsi = FormatSpec(fs, width = -1)
f = x->begin
ix = Integer(x)
io = IOBuffer()
_pf(io, fsi, ix, op)
String(take!(io))
end
s = fmt_Number(x, f)
_pfmt_s(out, fs, s)
end

function _pfmt_i(out::IO, fs::FormatSpec, x::Number, op::Op) where {Op}
_pfmt_Number_i(out, fs, x, op, _pfmt_i)
end

function _pfmt_f(out::IO, fs::FormatSpec, x::Number)
_pfmt_Number_f(out, fs, x, _pfmt_f)
end

function _pfmt_e(out::IO, fs::FormatSpec, x::Number)
_pfmt_Number_f(out, fs, x, _pfmt_e)
end

function fmt_Number(x::Complex, f::Function)
s = f(real(x)) * (imag(x) >= 0 ? " + " : " - ") * f(abs(imag(x))) * "im"
end
41 changes: 29 additions & 12 deletions src/fmtspec.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,25 +164,42 @@ _srepr(x) = repr(x)
_srepr(x::AbstractString) = x
_srepr(x::AbstractChar) = string(x)
_srepr(x::Enum) = string(x)
@static if VERSION < v"1.2.0-DEV"
_srepr(x::Irrational{sym}) where {sym} = string(sym)
end

function printfmt(io::IO, fs::FormatSpec, x)
cls = fs.cls
ty = fs.typ
if cls == 'i'
ix = Integer(x)
ty == 'd' || ty == 'n' ? _pfmt_i(io, fs, ix, _Dec()) :
ty == 'x' ? _pfmt_i(io, fs, ix, _Hex()) :
ty == 'X' ? _pfmt_i(io, fs, ix, _HEX()) :
ty == 'o' ? _pfmt_i(io, fs, ix, _Oct()) :
_pfmt_i(io, fs, ix, _Bin())
try
ix = Integer(x)
ty == 'd' || ty == 'n' ? _pfmt_i(io, fs, ix, _Dec()) :
Copy link
Member

Choose a reason for hiding this comment

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

Code smell here - shouldn't have a bunch of copy-paste.
The try should only be around the Integer conversion also.
Also, if the value cannot be converted to some Integer type, it should probably go directly to _pfmt_s.

Copy link
Author

@hhaensel hhaensel Dec 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I am afraid that I didn't completely get what you have in mind. We can't go directly to _pfmt_s, because we still need the formatting of the number part of the Number.

If we have a composed type like Complex{Int64} and we want to format it with 'd', this information needs to get passed on to the user-defined fmt_Number().

So my idea was:

  • we have _pfmt_i(out::IO, fs::FormatSpec, x::Integer, op::Op) where {Op} which does the normal formatting.
  • we add _pfmt_i(out::IO, fs::FormatSpec, x::Number, op::Op) where {Op} as a fallback which finally calls fmt_Number(x, f) where f contains _pfmt_i() with all the necessary parameters, so that the user simply needs to apply f() to the number part, e.g. f(real(x)).

Due to this fallback, I think, it is ok to accept ix in printfmt() to be either Integer or a number type with Integer number parts. Then we would get rid of the "code smell".

Example:
Integer(2.0) is 2.
Integer(2.0 + 3.0im) fails, so pyfmt("d", 2.0 + 3.0im) would fail. Therefore, fmt_Number() calls _pfmt_i(out, fs, x::Integer, op) on the real and imaginary part separately. So pyfmt("d", 2.0 + 3.0im) will finally result in 2 + 3im. If I would go directly to _pfmt_s, I would receive 2.0 + 3.0im.
Same story with _pfmt_f, but even more important, as e.g. float(2 - im) already acts on the number parts individually and results in 2.0 - 1.0im. So the type of fx in that case is always not predictable.

EDIT:
One comment on passing _pfmt_i in _pfmt_Number_i:
This would not be necessary, as the subtypes are covered by the op argument. whereas in the float case, where we have _pfmt_f, _pfmt_e and possibly later _pfmt_g. The subtype. Maybe, it's better to remove that.

Copy link
Member

@ScottPJones ScottPJones Dec 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

For complex numbers, you could use the defaults for "d" based on the type T in Complex{T},
no need to have to define a potentially unlimited number of "ComplexXXXX" types, just for setting defaults.
Because I added the capability of storing keyword information along with the formats, that also could be used to set how particular types of complex numbers are printed.

For example: in the default dictionary, have simply an entry Complex,
and add support for a keyword that control whether integers should be printed as integers.
I don't think anything more would be necessary.

I'm looking for methods that don't just simply solve your problem for printing complex numbers, but also handle it in a more generic, extensible fashion.

Copy link
Member

Choose a reason for hiding this comment

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

One thing you may not have noticed, the C-style formatted strings (i.e. f"\%f(var)" don't use the pfmt* functions, they use cfmt, which generates a formatter using @printf, so it won't use the pfmt functions you've been modifying.

ty == 'x' ? _pfmt_i(io, fs, ix, _Hex()) :
ty == 'X' ? _pfmt_i(io, fs, ix, _HEX()) :
ty == 'o' ? _pfmt_i(io, fs, ix, _Oct()) :
_pfmt_i(io, fs, ix, _Bin())
catch
ty == 'd' || ty == 'n' ? _pfmt_i(io, fs, x, _Dec()) :
ty == 'x' ? _pfmt_i(io, fs, x, _Hex()) :
ty == 'X' ? _pfmt_i(io, fs, x, _HEX()) :
ty == 'o' ? _pfmt_i(io, fs, x, _Oct()) :
_pfmt_i(io, fs, x, _Bin())
end
elseif cls == 'f'
fx = float(x)
if isfinite(fx)
ty == 'f' || ty == 'F' ? _pfmt_f(io, fs, fx) :
ty == 'e' || ty == 'E' ? _pfmt_e(io, fs, fx) :
try
Copy link
Member

Choose a reason for hiding this comment

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

Same as above - if it can't get converted to something that is AbstractFloat, then it should output as string, or give an error.

fx = float(x)
if isfinite(fx)
ty == 'f' || ty == 'F' ? _pfmt_f(io, fs, fx) :
ty == 'e' || ty == 'E' ? _pfmt_e(io, fs, fx) :
error("format for type g or G is not supported yet (use f or e instead).")
else
_pfmt_specialf(io, fs, fx)
end
catch
ty == 'f' || ty == 'F' ? _pfmt_f(io, fs, x) :
ty == 'e' || ty == 'E' ? _pfmt_e(io, fs, x) :
error("format for type g or G is not supported yet (use f or e instead).")
else
_pfmt_specialf(io, fs, fx)
end
elseif cls == 's'
_pfmt_s(io, fs, _srepr(x))
Expand Down
1 change: 1 addition & 0 deletions src/formatexpr.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ function printfmt(io::IO, fe::FormatExpr, args...)
end
end
isempty(fe.suffix) || print(io, fe.suffix)
nothing
end

const StringOrFE = Union{AbstractString, FormatExpr}
Expand Down
21 changes: 19 additions & 2 deletions test/fmt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,19 @@ i = 1234567
@test fmt(i) == "1234567"
@test fmt(i,:commas) == "1,234,567"

@test_throws ErrorException fmt_default(Real)
@test_throws ErrorException fmt_default(Complex)
@test fmt(2 - 3im, 10) == " 2 - 3im"
@test fmt(pi - 3im, 15, 2) == " 3.14 - 3.00im"

@test fmt(3//4, 10) == " 3//4"
@test fmt(1//2 + 6//2 * im, 15) == " 1//2 + 3//1*im"

fmt_default!(Rational, 'f', prec = 2)
fmt_default!(Format.ComplexRational, 'f', prec = 2)

@test fmt(3//4, 10, 2) == " 0.75"
@test fmt(3//4, 10, 1) == " 0.8"
@test fmt(1//2 + 6//2 * im, 23) == " 0.500000 + 3.000000im"
@test fmt(1//2 + 6//2 * im, 15, 2) == " 0.50 + 3.00im"

fmt_default!(Int, :commas, width = 12)
@test fmt(i) == " 1,234,567"
Expand All @@ -41,3 +52,9 @@ fmt_default!(UInt16, 'd', :commas)
fmt_default!(UInt32, UInt16, width=20)
@test fmt(0xfffff) == " 1,048,575"

v = pi
@test fmt(v) == "π"
@test fmt(v; width=10) == " π"

v = MathConstants.eulergamma
@test fmt(v, 10, 2) == " γ"
14 changes: 14 additions & 0 deletions test/fmtspec.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,17 @@ end
@test pyfmt("*>5f", Inf) == "**Inf"
@test pyfmt("⋆>5f", Inf) == "⋆⋆Inf"
end

@testset "Format Symbols (S) for Irrationals" begin
@test pyfmt(">10s", pi) == " π"
@test pyfmt("10s", pi) == "π "
@test pyfmt("3", MathConstants.eulergamma) == "γ "
@test pyfmt("10.2f", MathConstants.eulergamma) == " 0.58"
@test pyfmt("<3s", MathConstants.e) == "ℯ "
end

@testset "Format Symbols (S) for Irrationals" begin
pyfmt("10s", 3//4) == "3//4 "
pyfmt("10", 3//4) == " 3//4"
pyfmt("10.1f", 3//4) == " 0.8"
end