Open ben-schulz opened 4 years ago
Perhaps the possible fixes are in three layers.
Table
)Table(m::Integer, n::Integer, y_focus::UnitRange{<:Integer}, x_focus::UnitRange{<:Integer}; ...
)Considering the compatibility with (earlier) Compose.jl, it seems better to take 1. or 2. first.
Here's a simple "Compose-only" example on a 64-bit system:
x_focus = UnitRange{Int32}(1:3)
y_focus = UnitRange{Int32}(1:1)
Compose.Table(3,1, x_focus, y_focus, x_prop=ones(Float32,3), y_prop=ones(Float32,1))
MethodError: no method matching Compose.Table(::Int64, ::Int64, ::UnitRange{Int32}, ::UnitRange{Int32}; x_prop=Float32[1.0, 1.0, 1.0], y_prop=Float32[1.0])
I'd say this is a bug that's been introduced when upgrading Compose, so point 3 above is the solution for Compose.
FWIW, "git blame" says this is a potential issue from the beginning. Of course, I think modifying Compose.jl is fine.
Wondering what the root cause is here in the 32-bit Gadfly example above. E.g. is Gadfly or one of it's dependencies mistakenly forcing Int64
(giving rise to UnitRange{Int64}
) when it should be using Int
. The suspect UnitRange "values" originate in this line, so e.g. what is the Integer type of a length of an empty or non-empty DefaultDict
?
This is a topic of Gadfly, not Compose, but I guess DefaultDict
is innocent.
I think the problematic call of table
is here:
https://github.com/GiovineItalia/Gadfly.jl/blob/aec6dad99a4780fb61e409c5b5a591b66c4b728b/src/geom/subplot.jl#L252-L269
I think this line in Gadfly is the issue, because that's where the xgroup
field in the aes
struct gets created. That seems more of a bug than ::UnitRange{Int}
in Compose.
Your point seems to be correct.:+1:
However, it seems to have been changed to Int64
for some intent. I don't know the intent, though.
https://github.com/GiovineItalia/Gadfly.jl/pull/1129
https://github.com/GiovineItalia/Gadfly.jl/pull/1129/commits/1bfe85629cb0724b9d1428f67afd01a4c688a829
@andreasnoack your change of Int
to Int64
(see comment above) breaks Gadfly on 32-bit systems. can you explain why this was needed?
Unfortunately, I don't remember why I made that change and I agree that it looks wrong to use Int64
instead of Int
.
The index of a IndirectArray
should always be an integer:
data = Gadfly.Data(shape=["A","A", "B", "C","C"])
scale = Scale.shape_discrete()
disc_data = Scale.discretize(getfield(data, :shape), scale.levels, scale.order)
disc_data.index # are integers
Can test to see if removing the Int() here causes any errors in Gadfly.
The problem line above got changed in GiovineItalia/Gadfly.jl#1411, so I assume the OP is fixed.
@ben-schulz can you test if this now works on master?
when building the docs for [Gadfly.jl]() using v1.3.1 of the 32-bit Julia binary for Linux, i get the following error:
steps to reproduce:
full stack trace:
(originally referenced in: https://github.com/GiovineItalia/Gadfly.jl/issues/1374#issuecomment-576054508)