mwouts / itables

Pandas DataFrames as Interactive DataTables
https://mwouts.github.io/itables/
MIT License
757 stars 56 forks source link

first very naive version of integrating buttons #18

Closed jpbarrette closed 2 years ago

jpbarrette commented 3 years ago

This is a very naive version that integrates basic functionalities for buttons' extentions. I don't expect this to be merged, just give me guidance for getting this integrated.

codecov-io commented 3 years ago

Codecov Report

Merging #18 (1ac61aa) into master (87a5614) will increase coverage by 0.27%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   96.77%   97.04%   +0.27%     
==========================================
  Files          10       10              
  Lines         217      237      +20     
==========================================
+ Hits          210      230      +20     
  Misses          7        7              
Impacted Files Coverage Δ
itables/__init__.py 100.00% <ø> (ø)
itables/javascript.py 94.05% <100.00%> (+1.37%) :arrow_up:
itables/options.py 100.00% <100.00%> (ø)

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 87a5614...1ac61aa. Read the comment docs.

mwouts commented 3 years ago

Hello @jpbarrette , thank you for submitting this! I'll give a try to your PR in the next days. It would be a good opportunity for me to come back to this project (it has been a while since I implemented the first version). I'll also see how to best answer your question.

mwouts commented 3 years ago

Oh interesting! Many thanks for the example. I've taken this screenshot Screenshot from 2020-11-13 00-56-52

Do you think it would be possible / nice to being able to use it, like, say:

import itables.options as opt
opt.extensions = ["buttons"]

show(df, buttons = ['csvHtml5'])

or even, if we want it for all the dataframes:

import itables.options as opt
opt.extensions = ["buttons"]
opt.buttons = ['csvHtml5']
import itables.interactive

df

?

jpbarrette commented 3 years ago

Yes, I think it's a good idea. I'll check what I can do.

mwouts commented 3 years ago

Hi @jpbarrette , I was having a look at your changes, and must say that I like very much the direction of your second commit - allowing to load extensions in a generic way is certainly the way to go.

Maybe we could have a dict that references the latest url for all the existing extensions, and load them only on demand, according to an extensions argument which we would add to show ?

Also, I think that the function load_datatables is currently the weak part of the project. Maybe your PR could be also an opportunity to chat about or even revisit the logic for loading the JS library. Currently load_datatables injects the JS code to load the datatables.net library if it was not loaded previously, but... previously here refers to a global variable in the Python kernel, which is not always relevant (and may probably be the cause of #6 and #17). I'll see if I can think of a better way (and also look at how Plotly do this).

mwouts commented 3 years ago

I have added a short extract of a Plotly notebook at https://github.com/mwouts/itables/blob/4fbf5bb3351e21755bf8c1eeb531141b4bfd7103/experiments/small_plotly_graph.html

Is this of any help? I think this could be a leading example as Plotly works well in any context (jupyter notebook, nbconvert, and also lab, but then it requires the installation of a jupyterlab extension).

mwouts commented 3 years ago

Hi @jpbarrette , I did some experiments with load_datatables and also with Plotly, and I think I'd be in favor of moving the call to load_datatables from __init__.py to inside the _datatables_repr_ function, cf. https://github.com/mwouts/itables/issues/17#issuecomment-727556778. Maybe we could use a global variable like you did to detect if the library has been loaded already - or even to keep track of which extensions have been loaded yet.

Also, is it correct that we can't load a JS library twice? Alternatively, do you think we could use a JS variable to keep track of which libraries/extensions we have loaded yet, so that each table display would load dynamically the ones that are required?

jpbarrette commented 3 years ago

I think I agree with most of your suggestions, but we should do it in different steps:

  1. As you mentionned in https://github.com/mwouts/itables/issues/17#issuecomment-727556778, we should document that load_datatables() must be called and remove it from __init__.py. But let's not completely refactor load_datatables() yet.

  2. From your comment:

    Maybe we could have a dict that references the latest url for all the existing extensions, and load them only on demand, according to an extensions argument which we would add to show ?

    What I like from your library, is the fact that it's a lightweight proxy to datatables.net. Sure, if you want to do a simple thing, you can get your table displayed easily, but if you want something more complex, datatables.net documentation is there for that. Essentially my take is: If you want to simplify datatables' usage, I would rather code that in the original library, or do a JS library that simplifies the process (which you would have a proxy for in itables).

  3. Before refactoring load_datatables, we need to understand the following:

    • Is there other issues than https://github.com/mwouts/itables/issues/17#issuecomment-727556778 that you want to fix at the same time?
    • Do you want this library to work only on jupyter, or on other notebooks also? Plotly with its "renderers", can handle a lot of different notebooks, but it's pretty complex. Looking at their code, I suspect that Plotly "rendering" part could be a separate library that could be of use for itable and others (ie. it would be a huge lost of time to recode the same things here). The other solution would be to use something like jupyter_require, even though I don't agree with its "Execution & Security -- safe scripts and finalization" statement that it relies on (But I might just be completely wrong here, so I'm tagging @CermakM so that he can give more details if he wants)
    • Can you expand on your intention/design philosophy for this library?
mwouts commented 3 years ago

Can you expand on your intention/design philosophy for this library?

The project was much inspired by DT. When I switched from R to Python I was wondering why I couldn't use the same interactive tables in all these contexts:

Currently itables only works in notebook and HTML. I think adding support for Jupyter Lab should be feasible. Adding support for Dash applications seems to be much above my JS skills, but datatables (or aggrid, etc) seem to be more powerful than their current datatable component.

For me it is important that the interactive representation of tables is both robust and useful. I mean, whatever dataframe the user has, itables should never freeze the notebook or make it too big. And the JS table should always offer more than the default pandas representation. For instance, I often find that pandas dataframes are way to long by default, and force me to scroll in notebooks. What I expect from itables are more concise tables, and if necessary the user can search or scroll in the table itself. I am not so sure that itables works that well with big tables with complex multiindex (rows or columns).

Also, as you mentioned, an idea inherited from DT is that users with knowledge of the underlying javascript library can customise the rendering of their tables.

Finally, I have not touched the 'server side rendering' but I would be interesting in experimenting that direction (probably in Jupyter Lab). I'd like to embed just a short extract of the table data in the notebook itself, and use a Jupyter server extension to serve the table data. And let the user choose, of course, if they want to export the full data in the notebook, for the HTML export use case.

mwouts commented 2 years ago

Hi @jpbarrette , the project has changed in a significant way since we started this PR. In particular

So if you don't mind I am going to close this issue and keep a link on it at #50