-
Notifications
You must be signed in to change notification settings - Fork 38
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
max_num_of_rows not taking effect in html backend #217
Comments
Hi @juliohm ! Everything if fine here, how about you? Can you provide a MWE? Because this option is working as expected here and also in DataFrames.jl. Take a look at this example: julia> a = rand(10_000, 10);
julia> @time pretty_table(IOBuffer(), a; tf = tf_html_default, max_num_of_rows = 10)
0.078409 seconds (350.23 k allocations: 24.639 MiB, 99.56% compilation time)
julia> @time pretty_table(IOBuffer(), a; tf = tf_html_default, max_num_of_rows = -1)
0.178180 seconds (4.59 M allocations: 228.235 MiB) PrettyTables.jl uses the same internal handling (a type I call Can it be something specific to your type that somehow triggers the entire rendering when a cell is called with |
The last time I saw something like this was caused due to the Tables.jl implementation of the type, that was leading to entire column rendering. Take a look here please: |
I am doing fine, thanks! :) Here is a MWE that you can try on Jupyter (I tried it on VSCode directly because it supports notebooks): using GeoTables
georef((a=rand(1000,1000),)) It takes 43s for me. |
Here is the precise location of the call: |
I think it is our fault! Splatting on the _common_kwargs is causing the problem 🤦🏽♂️ |
But I don't know why. The _common_kwargs function returns very small objects, it shouldn't be a bottleneck. |
@ronisbr can you please confirm the semantics of the option Apparently, if we set the maximum number to 10 and the table has 1000000 rows, no information is shown to indicate the omitted rows: |
Hi @juliohm ! Yes, this is the expected behavior. There is some room for improvement when using The advice is to use I will check why it is taking too much time in that function. |
Hi @juliohm ! I understood the problem! When you use Line 106 in b72c085
This process is taking too long for GeoTables. If you change The vast majority of the time, the algorithm is in this function here: |
Thank you @ronisbr ! Implementing the suggestions now. Will report back if the issue is still present. Feel free to change the title of the issue to a more appropriate name :) |
Thanks for the report! I think I can greatly improve everything if I keep a cache of the state. I will try to do some modifications here and hope it will not introduce a lot of type instabilities. My ideia is:
This algorithm will greatly improve things for this case. However, my concern is that it will allocate a lot more for the other scenarios in which the iteration is much faster. |
Hi @juliohm! I could reduce the time from several seconds to 0.001s considering that GeoTables state in the iteration is the row number. I think we should open a issue in Tables.jl to add a information that a RowTable state is the row number, which will drastically reduce the iterations, leading to a substancial gain. |
By the way, if Tables.jl did not add this hint, I will add a keyword probably called: |
Thank you Ronan! That is very helpful! This package is truly amazing and
only gets better!
Em qua., 6 de set. de 2023 12:44, Ronan Arraes Jardim Chagas <
***@***.***> escreveu:
… By the way, if Tables.jl did not add this hint, I will add a keyword
probably called: row_tables_jl_use_row_id_as_state, which will solve the
problems for GeoTables!
—
Reply to this email directly, view it on GitHub
<#217 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZQW3P5Q7OIOTY2B4OXCGDXZCK4JANCNFSM6AAAAAA4MOJ5AA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I think this issue is solved with the new code that uses |
Hi @ronisbr , hope you are doing well,
We are facing an issue with the html backend because it is materializing entire columns even when we pass the option
max_num_of_rows
. Is there a quickfix or workaround to avoid building the entire columns in this backend?The text backend is working just fine and we can create very large tables and display just a few rows quickly. The problem is in the creation of the objects, not the display itself.
The text was updated successfully, but these errors were encountered: