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

nbShow for images and tables #74

Closed
pietroppeter opened this issue Nov 17, 2021 · 18 comments · Fixed by #179
Closed

nbShow for images and tables #74

pietroppeter opened this issue Nov 17, 2021 · 18 comments · Fixed by #179
Assignees
Labels
2023H1 enhancement New feature or request
Milestone

Comments

@pietroppeter
Copy link
Owner

we should have a dedicated command nbShow whose behaviour will depend on the type of object is applied.

For example:

  • when applied to a ggplot, it should show the image (would it be possible/advisable to save as svg and incorporate directly in html?)
  • when applied to a dataframe, it should print a nice table

this should be in some nimib/show module that should uses when compiles(import ggplotnim) or similar mechanism not to introduce additional dependencies.

I plan to do this after #24 (hopefully I will start soon working on that, likely starting from scratch)

@HugoGranstrom
Copy link
Collaborator

this should be in some nimib/show module that should uses when compiles(import ggplotnim) or similar mechanism not to introduce additional dependencies.

So it would only work for types that are officially supported by nimib? I don't have full understanding of how this works, but wouldn't something like this perhaps work?

template nbShow(t: untyped) =
	t.nbShowApi(nbDoc)

proc nbShowApi(m: MyCustomType, nb: NbDoc) =
	# Logic here

All types which implement nbShowApi are then supported. There might be aspects I'm missing of course which require tight nimib integration 😄

@pietroppeter
Copy link
Owner Author

Still too early for me to understand how it will be for a custom type, but there will be some mechanism like the one you suggest to make this something that any custom type can implement.

I do think though that for each nbShow there might be custom parameters (e.g. max rows and max columns for a table, caption for a plot, ...).

@HugoGranstrom
Copy link
Collaborator

I do think though that for each nbShow there might be custom parameters (e.g. max rows and max columns for a table, caption for a plot, ...).

That's a very good point! Didn't think about that.

@pietroppeter
Copy link
Owner Author

have been thinking about how this could be implemented, here I describe a possible implementation:

  1. change partial for nbCode from
nb.partials["nbCode"]= """
{{>nbCodeSource}}
{{>nbCodeOutput}}
{{>nbCodeHtmlOutputs}}
"""
nb.partials["nbCodeHtmlOutputs"]= """
{{#htmls}}{{.}}{{/htmls}}
"""
  1. we provide a addHtml(blk: var NbBlock, html: string) function that adds a generic html string to current block (initializes sequence htmls if not present.

  2. a generic nbShow[T](obj: T) template is provided that calls a T.toHtml function and uses addHtml to add output to current block.

  3. for every specific type that ones want to show, you need to implement toHtml(o: MyType): string =

  4. (optional) we might be able to pass some untyped varargs argument to nbShow that are passed as is to toHtml to implement options (like maxRows, maxCols, ...). If we are not able than to use non default of optional arguments one needs also to specialize nbShow and pass arguments to toHtml (a macro could be supplied to do that automatically).

what is nice about the above is that for every new type you just need to add a toHtml function that might be anyway useful besides nimib. Also I liked the use of addHtml that hides the implementation detail of how the partials are handled. For example if in a next version of nimib we add a partial field to the block object (so that you can customize the single block), then the addHtml will change its implementation but not the rest.

Note that in the above we assume that all html outputs produced by nbShow are shown after the capture stdout outputs. It would be complex to do otherwise and not worth the effort IMO.

@HugoGranstrom
Copy link
Collaborator

This API looks really clean, I really like the toHtml idea 😁 the only nitpick I have is that nbShow and --nbShow could be a bit confusing for those uninitiated. Not sure what else to call it though 🤔

@pietroppeter
Copy link
Owner Author

ah yes, all names are up for changes:

  • nbShow: an alternative could be nbView
  • nbCodeHtmlOutputs: why specific to nbCode blocks? maybe just nbHtmlOutputs? or maybe we lose the nb prefix since it is not supposed to be tied to a specific block? e.g. htmlChunks?
  • htmls: as name of field in the context/data. are there better options here?

@HugoGranstrom
Copy link
Collaborator

Here are my 10 cent:

  • nbView isn't bad. Throwing nbPretty into the mix as well.
  • I think the nb prefix is suitable still, nbHtmlOutputs sounds fine to me.
  • I actually think htmlChunks fits better for this than the above. htmls feels a bit too vague of a name and something that a user might create in their own contexts/partials.

@pietroppeter
Copy link
Owner Author

Inflation really is ramping up 😜

  • Yeah to be honest I still think nbShow is the better option (should we rename the runtime option to nbOpen?)
  • agree on htmlChunks but I was trying to avoid 🙄 the decision whether to use html_chunks (as we do for many theme related things) or htmlChunks. Do we have a style insensitive json?

@HugoGranstrom
Copy link
Collaborator

🤣 That's what I get when I don't know your idioms properly...

  • Fair enough, npOpen is more logical for the runtime option actually 👍
  • Hehe, I'd go with the convention we currently have when in doubt. Style insensitive json doesn't sound like something someone would want 😅

@HugoGranstrom
Copy link
Collaborator

I have thought a bit about this now. Why must nbShow be attached to a nbCode? Can't we just make it an independent block? For example, I might want to nbShow a dataframe without showing any code. Then I should just be able to do nbShow(df) anywhere in my document. nbShow would in this case simply be:

template nbShow(obj: untyped) =
  nbRawHtml(obj.toHtml())

Thought on this? Am I missing something we thought was brilliant with your approach back in October?

@pietroppeter
Copy link
Owner Author

Well, this might just be more brilliant :) This is rather straightforward indeed.

The idea I had back then was to be able to use it inside nbCode so that you could show how you are calling it but indeed it is not very important (and maybe this actually works inside nbCode - the only issue would be that you cannot anymore change nbCode as last block - for example calling nbClearOutput)

@HugoGranstrom
Copy link
Collaborator

I don't think it works to create other blocks inside nbCode (that's why we created nimibCode). So it would have to be done afterwards. The only time I can see this being a problem is with nbCodeInBlock as the variable wouldn't be available afterwards. But then you could wrap a nbCode manually in a block and do the nbShow inside the block.
But I do see your point that it would be nice to actually show in the code that you are printing the dataframe for example so it doesn't appear from nowhere. But for that to work we would need to use nimibCode which if I recall don't capture the output currently.

@HugoGranstrom
Copy link
Collaborator

And to recap nbCode vs niminCode:

#nbCode
newBlock(....)
  capturestdOut: 
    body

#nimibCode
newBlock(....): # just read the code here, don't run it
  discard
body # we run it afterwards instead

Looking at this I think we could make nimibCode captureStdout if we just save a reference to the newly created block and add the output to it. :o If I could get it to work, maybe we could change nbCode to use this and deprecate nimibCode as it could handle nesting now? 🤔

@pietroppeter
Copy link
Owner Author

Well, with support for container blocks (and allowing every block to be a container block) I think we should be able to call blocks from inside other blocks.

@pietroppeter
Copy link
Owner Author

the ref issue for container blocks is #117 and if they support also this use case we should actually make any block a container block

@HugoGranstrom
Copy link
Collaborator

That's a good point, all the contained blocks of a nbCode could be placed where you put the >nbCodeHtmlOutputs in your example above.

@pietroppeter
Copy link
Owner Author

pietroppeter commented Feb 9, 2023

ah the part of the partial I had not yet thought about it... to be honest it is late and I am really tired so there are many things that I might not have considered, but making blocks also container blocks is something that looks more and more appealing!

@HugoGranstrom
Copy link
Collaborator

Haha yeah it's a shame that one has the most time when you should sleep and is tired 🤣
I'll sleep on this as well. This is not a blocker for nbShow currently though so I will proceed with it and we'll have the discussions on containers in that issue. When we do implement them though, nbShow will be even more useful as it could be used inside nbCode as well :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2023H1 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants