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

Redundant rendering method for DataFrames #224

Open
essenciary opened this issue Sep 21, 2023 · 5 comments
Open

Redundant rendering method for DataFrames #224

essenciary opened this issue Sep 21, 2023 · 5 comments

Comments

@essenciary
Copy link
Member

essenciary commented Sep 21, 2023

We're rendering Tables and DataFrames are tables - and the rendering methods produce the same output.

I'll remove the DataFrames renderer.

@hhaensel Lmk if I'm missing something please.

@essenciary
Copy link
Member Author

Actually they are not identical on the Julia side - which is a bad thing and a good reason to have just one method.

julia> Stipple.rendertable(DATA)
OrderedDict{Symbol, AbstractVector} with 2 entries:
  :x1 => [4.79784e-6, 0.000333976, 0.000418498, 0.000479837, 0.000492418, 0.00080857, 0.000909854, 0.00118906, 0.00119842, 0.00120826    0.
  :x2 => [0.107271, 0.542181, 0.143818, 0.240444, 0.945631, 0.193178, 0.825941, 0.924873, 0.339736, 0.0126997    0.137574, 0.0738368, 0.970

julia> Stipple.render(DATA)
OrderedDict{String, AbstractVector} with 2 entries:
  "x1" => [4.79784e-6, 0.000333976, 0.000418498, 0.000479837, 0.000492418, 0.00080857, 0.000909854, 0.00118906, 0.00119842, 0.00120826    0
  "x2" => [0.107271, 0.542181, 0.143818, 0.240444, 0.945631, 0.193178, 0.825941, 0.924873, 0.339736, 0.0126997    0.137574, 0.0738368, 0.97

@hhaensel
Copy link
Member

You are indeed missing something. rendertable() is a renderer for types that support the Tables API. As Table is not an abstract type you can determine by istable() whether a datatype supports the Tables API. But there are some generic types that should not automatically be rendred as tables. (Actually first they were and we had to close an issue)
So now, if you want to add a table renderer for a special datatype, you can do that by calling rendertable().
For may other table types render() calls rendertable() that's why the results are identical

@essenciary
Copy link
Member Author

essenciary commented Sep 22, 2023

@hhaensel Thanks, I did check the code and (that's why) I still don't get it. I'm talking specifically about DataFrames. Why do we need a dedicated DataFrames renderer given that Tables.istable(df::DataFrame) === true?
Also, the results are not identical. One returns a Dict with String keys, other with Symbol keys. Basically the DataFrames renderer specializes the Tables renderer only to produce a Dict with Symbol keys. Why?

@hhaensel
Copy link
Member

Sorry, misread on my mobile. hat you were only talking about DataFrames, my bad.

I implemented a separate method for DataFrames because I thought it should be faster than rendering via the Tables API.
Just checked that this is true:

julia> @btime Stipple.rendertable(df)
  933.333 ns (11 allocations: 720 bytes)
OrderedDict{Symbol, AbstractVector} with 2 entries:
  :a => [1, 2, 3]
  :b => ["a", "b", "c"]

julia> @btime Stipple.render(df)
  501.235 ns (10 allocations: 688 bytes)
OrderedDict{String, AbstractVector} with 2 entries:
  "a" => [1, 2, 3]
  "b" => ["a", "b", "c"]

@hhaensel
Copy link
Member

hhaensel commented Nov 3, 2023

@essenciary So I propose we leave everything as is due to speed considerations.

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

No branches or pull requests

2 participants