search

jupyter-widgets / ipydatagrid

Fast Datagrid widget for the Jupyter Notebook and JupyterLab
BSD 3-Clause "New" or "Revised" License
575 stars 51 forks source link

py2vega future #119

Open martinRenou opened 4 years ago

martinRenou commented 4 years ago

cc. @kaiayoung @mbektasbbg @supriyakhandekar @ibdafna

@supriyakhandekar 's issue on py2vega made me rethink the py2vega scope https://github.com/QuantStack/py2vega/issues/36

Vega expressions are expressions, hence they do not support assignments, for-loops, while-loops, if-conditions.

I wanted to go around it and added fake assignment and if-condition support in py2vega. Which I thought was nice at first. But I fear there would be feature requests for for-loops, while-loops, class definitions and such, which will be far from trivial to implement. Its complexity could easily increase. Also, the pseudo if-condition support might bite users more than once.

I feel like we should maybe reduce the py2vega scope, and remove Python functions support (as well as assignment support). Supporting only lambdas and Python code strings (actual expressions) as input, which means only supporting code that can be executed with a Python eval call. This would make it clearer to the user what py2vega can and cannot do.

Please tell me what you think.

gaborbernat commented 3 years ago

Ping @kaiayoung @mbektasbbg @supriyakhandekar @ibdafna on the topic. Is this still the case, anything actionable here to do?

mbektas commented 3 years ago

@martinRenou I wonder if it is possible to detect such issues when/before executing the expression, as in QuantStack/py2vega#36 and output more useful error messages to user. A message specific to the source of error, mentioning limitations / a link to limitations document on ipydatagrid wiki page or similar.

martinRenou commented 3 years ago

I wonder if it is possible to detect such issues when/before executing the expression

I guess it could be possible yes. We could have some whitelisting of functions/variables, and if a function/variable is not whitelisted we could show a meaningful error message.