RandomFractals / vscode-data-table

Data Table 🈸 , Flat Data Grid 中 & Data Summary 🈷️ Renderers for VSCode Notebook 📓 Cell ⌗ Data Outputs
https://marketplace.visualstudio.com/items?itemName=RandomFractalsInc.vscode-data-table
Apache License 2.0
26 stars 5 forks source link

Extension displays multiple stdout stream outputs in polyglot notebooks #153

Closed colombod closed 1 year ago

colombod commented 1 year ago

Users are reporting this issue when the extension is installed. https://github.com/dotnet/interactive/issues/3053.

The presence of the extension causes this output: image

RandomFractals commented 1 year ago

@colombod probably because your C# code cell produces 3 cell outputs when that code runs, instead of one with:

line 0
line 1
line 2

Not sure how we can help you there. Perhaps, review your console logging and outputs produced by the .net code cells.

colombod commented 1 year ago

stdout is supposed to be concatenated to previous cell otherwise doing write instead of writeline will produce outputs on two lines instead, also that will cause the the output cell to have more than one output block. I am not sure about the requirement of intercepting stream output types for the fata-table renderers.

colombod commented 1 year ago

@colombod probably because your C# code cell produces 3 cell outputs when that code runs, instead of one with:

line 0
line 1
line 2

Not sure how we can help you there. Perhaps, review your console logging and outputs produced by the .net code cells.

@RandomFractals the quoted statement is not correct as the code produces exactly a since cell output element in the vscode extension. This is the line stream outputs are appended to the existing one, you can review this part : https://github.com/dotnet/interactive/blob/2ca13bab0c1a4fba095802d6e5d9409112ce305d/src/polyglot-notebooks-vscode-common/src/notebookControllers.ts#L269

The code of polyglot notebook is not producing 3 outputs but appending stream types to the latest one.

Is it possible to reopen this issue?

Thank you.

RandomFractals commented 1 year ago

@colombod do you have a link to the standard "streamed" output in VS Code notebooks examples or documentation to resolve it, without the special and custom .net interactive, now "polyglot" notebooks internals?

colombod commented 1 year ago

No but that code is what the developers of jupyter extension advices to do. Can see if I can find more info for you but how to handle outputs is the responsibility of the notebook controller and it depends on requirements . For us was to be compliant with expectations from jupyter lab and python users

vityas commented 1 year ago

Referencing an issue I asked about in an extension that required data tables: https://github.com/paiqo/Databricks-VSCode/issues/163

short story, installing this extension prevents cells that have print statements to behave as expected.

gbrueckl commented 1 year ago

has there been any update to this @RandomFractals? As this is affecting all users that installed our Databricks Extension due to the dependency we have currently added to this extension

RandomFractals commented 1 year ago

I will look into patching it this week, but as you noted in your ticket there is a workaround to simply use the built-in renderer for such outputs.

RandomFractals commented 1 year ago

@gbrueckl & @colombod after looking more into this, I am tempted to remove my renderers from supporting application/vnd.code.notebook.stdout mime type, and let vscode handle it. The long text cell output rendering is much better in vscode now with the built-in output renderer. That was the only reason I added it to view scrollable text when renderers api came out and built-in renderer did not handle long outputs well.

Related config: https://github.com/RandomFractals/vscode-data-table/blob/main/package.json#L96

@gbrueckl does your extension and databricks users use stdout or other text and json data mime types to view tabular data with my data table renderers?

gbrueckl commented 1 year ago

no, we are not using stdout at all, we mainly use JSON and sometimes plain text

we use text/html, text/plain, text/x-json, application/json, application/databricks-table and text/markdown

colombod commented 1 year ago

@gbrueckl & @colombod after looking more into this, I am tempted to remove my renderers from supporting application/vnd.code.notebook.stdout mime type, and let vscode handle it. The long text cell output rendering is much better in vscode now with the built-in output renderer. That was the only reason I added it to view scrollable text when renderers api came out and built-in renderer did not handle long outputs well.

Related config: https://github.com/RandomFractals/vscode-data-table/blob/main/package.json#L96

@gbrueckl does your extension and databricks users use stdout or other text and json data mime types to view tabular data with my data table renderers?

I think this will avoid the issue where stream are printed multiple times.

RandomFractals commented 1 year ago

Great! @gbrueckl & @colombod thanks for the quick follow up.

I'll remove stdout mime types from the 3 data table renderers, and hopefully that will make a much better experience for both new polyglot and databricks notebook users.

Will try to push that update this week. Thanks for your patience on this!

RandomFractals commented 1 year ago

so, a few things that would help us reproduce scenarios like this next time:

  1. @colombod please share your test .ipynb next time.
  2. Would be nice if we could comment on your original issue in your repo: https://github.com/dotnet/interactive/issues/3053 (currently can't on any of the "Open at Microsoft" repos :( cc @jonsequitur
  3. Your Open at Microsoft org should sponsor our extensions and issues resolutions like this as mentioned in the past since most of your new polyglot users now use it too because new Data Wrangler requires Python install and not all devs want to use it with Jupyter notebooks in code for simple tabular data display and fancy renderings our Data Table renderers provide. cc @jonsequitur

Created this Jupyter notebook to test it out:

image

Output rendering with built-in renderer:

image