Rewrite of tables. #58

tpapp · 2018-02-07T15:48:18Z

Table code:

A step in fixing #44. Complete rewrite of Table. First argument is options, which is optional, the rest is dispatched to various conversion methods. Previous constructors are kept, extended with matrix/edge, named pairs, etc. Printing rewritten, scanlines made explicit.

Other, related code:

factored out matrix edge formatting into a common function, used by tables and coordinates
print NaN and +-Inf properly (pgfplots can use it)
Contour and Histogram are now converted into tables with Table.
changed testing for whitespace, introduced a simple field comparison in tests

Examples/gallery:

added a new file for various table constructors, and how to omit points,
changed syntax of Table calls accordingly,
added examples for histograms

Accidental fix:

make print_tex work for AbstractString, it failed on a CategoricalString from a dataframe in the gallery

Table code: Complete rewrite. First argument is options, which is optional, the rest is dispatched to various conversion methods. Previous constructors are kept, extended with matrix/edge, named pairs, etc. Printing rewritten, scanlines made explicit. Other, related code: - factored out matrix edge formatting into a common function, used by tables and coordinates - print NaN and +-Inf properly (pgfplots can use it) - Contour and Histogram are now converted into tables with Table. - changed testing for whitespace, introduced a simple field comparison in tests Examples/gallery: - added a new file for various table constructors, and how to omit points, - changed syntax of Table calls accordingly, - added examples for histograms Accidental fix: - make `print_tex` work for `AbstractString`, it failed on a `CategoricalString` from a dataframe in the gallery

codecov-io · 2018-02-07T15:55:23Z

Codecov Report

Merging #58 into master will increase coverage by 2.97%.
The diff coverage is 90.78%.

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   74.94%   77.91%   +2.97%     
==========================================
  Files           8        8              
  Lines         443      471      +28     
==========================================
+ Hits          332      367      +35     
+ Misses        111      104       -7

Impacted Files	Coverage Δ
src/utilities.jl	`92.45% <100%> (+0.61%)`	⬆️
src/PGFPlotsX.jl	`58.33% <87.5%> (+8.33%)`	⬆️
src/axiselements.jl	`91.39% <89.79%> (+2.42%)`	⬆️
src/requires.jl	`84.84% <93.33%> (+22.34%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4d07af...b685411. Read the comment docs.

KristofferC · 2018-02-07T16:28:19Z

src/requires.jl

+    end
+
+    function PGFPlotsX.table_fields(histogram::StatsBase.Histogram{T, 2}) where T
+        PGFPlotsX.table_fields(midpoints(histogram.edges[1]),


midpoints seems deprecated from looking at the generated docs:

WARNING: midpoints(r::Range) is deprecated, use r[1:length(r) - 1] + 0.5 * step(r) instead. Stacktrace: [1] depwarn(::String, ::Symbol) at ./deprecated.jl:70 [2] midpoints(::StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}) at ./deprecated.jl:57 [3] table_fields(::StatsBase.Histogram{Int64,2,Tuple{StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}},StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}) at /Users/kristoffer/Dropbox/julia/PGFPlotsX/src/requires.jl:62 [4] #Table#42(::Array{Any,1}, ::Type{T} where T, ::StatsBase.Histogram{Int64,2,Tuple{StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}},StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}, ::Vararg{StatsBase.Histogram{Int64,2,Tuple{StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}},StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}},N} where N) at /Users/kristoffer/Dropbox/julia/PGFPlotsX/src/axiselements.jl:458 [5] PGFPlotsX.Table(::StatsBase.Histogram{Int64,2,Tuple{StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}},StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}) at /Users/kristoffer/Dropbox/julia/PGFPlotsX/src/axiselements.jl:458 [6] eval(::Module, ::Any) at ./boot.jl:235

KristofferC · 2018-02-07T16:29:42Z

test/runtests.jl

@@ -2,6 +2,7 @@ using PGFPlotsX
 using Base.Test
 using Compat
 using DataStructures: OrderedDict
+using DataFrames


We should make sure to using all deps here because otherwise we get recompilation messages in the generated documentation.

OK, will do.

KristofferC · 2018-02-07T16:36:28Z

src/axiselements.jl

+    $SIGNATURES
+
+Named columns provided as a vector of pairs, eg `[:x => 1:10, :y => 11:20]`.
+Symbols or strings are accepted as column names.


Is this enforced anywhere?

Yes, currently in the method dispatch.

KristofferC · 2018-02-07T16:39:21Z

src/axiselements.jl

-            print(io, s[1], "    ")
-            push!(vs, s[2])
-        end
+struct TableFile <: OptionType


What's the reason over just e.g. Table(path).

You mean Table(path) constructing a TableFile, or simply storing the path in a Table?

I have in mind an interface for writing Table data into files and returning an appropriate TableFile, ie externalizing tables, for larger datasets, but plan to explore this later.

I am now convinced that it is best if we keep very close to the syntax of pgfplots, so I modified code to use the Table constructor for these. Thanks!

KristofferC · 2018-02-07T16:40:39Z

src/requires.jl

            end
        end
+        hcat(colx, coly, colz), ["x", "y", "z"], cumsum(ns)
    end
 end

 # TODO: Check if the bins are completely correct


Can this comment be removed?

KristofferC · 2018-02-07T16:42:10Z

docs/src/examples/tables.md

+
+![](table-named-columns.svg)
+
+or a dictionary, here renamed using options:


What's the reason for including a Dict constructor. Since the ordering is unknown, is that a good idea?

1. Removed `<: Associative` constructor for `Table`, since the order may not be well defined. Removed `Dict` from examples. 2. Introduced a workaround for midpoints, as their status is unclear ATM. 3. `using` packages we need to generate the manual, to suppress compilation messages.

This keeps the vocabulary of this library close to `pgfplots`. Also convert paths to absolute if necessary, otherwise they will not be found when compiling from temporary files.

KristofferC · 2018-02-08T13:20:10Z

src/axiselements.jl

 """
-TableFile(path::AbstractString) = TableFile(OrderedDict{Any, Any}(), path)
+Table(options::OrderedDict{Any, Any}, path::AbstractString) =


It is a tiny bit weird that the Table constructor does not return a Table. However, in practice, I don't think it matters.

This allows dispatching print_tex on the type.

If this was the last issue and you don't mind it this way, can we merge?

Sure, go ahead.

tpapp mentioned this pull request Feb 7, 2018

NaN and Inf in Coordinates #59

Closed

KristofferC reviewed Feb 7, 2018

View reviewed changes

tpapp added 2 commits February 8, 2018 09:40

Use the Table constructor for TableFile's.

b685411

This keeps the vocabulary of this library close to `pgfplots`. Also convert paths to absolute if necessary, otherwise they will not be found when compiling from temporary files.

KristofferC reviewed Feb 8, 2018

View reviewed changes

KristofferC approved these changes Feb 9, 2018

View reviewed changes

tpapp merged commit 4211f29 into master Feb 9, 2018

tpapp deleted the tp/table-rewrite branch February 9, 2018 13:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite of tables. #58

Rewrite of tables. #58

tpapp commented Feb 7, 2018

codecov-io commented Feb 7, 2018 •

edited

Loading

KristofferC Feb 7, 2018

KristofferC Feb 7, 2018

tpapp Feb 7, 2018

KristofferC Feb 7, 2018

tpapp Feb 7, 2018

KristofferC Feb 7, 2018

tpapp Feb 7, 2018

tpapp Feb 8, 2018

KristofferC Feb 7, 2018

tpapp Feb 7, 2018

KristofferC Feb 7, 2018

KristofferC Feb 8, 2018

tpapp Feb 9, 2018

KristofferC Feb 9, 2018


		![](table-named-columns.svg)

		or a dictionary, here renamed using options:

Rewrite of tables. #58

Rewrite of tables. #58

Conversation

tpapp commented Feb 7, 2018

codecov-io commented Feb 7, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 7, 2018 •

edited

Loading