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

Code cell in blockquote does not execute #1134

Closed
rowanc1 opened this issue Apr 18, 2024 · 3 comments
Closed

Code cell in blockquote does not execute #1134

rowanc1 opened this issue Apr 18, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@rowanc1
Copy link
Member

rowanc1 commented Apr 18, 2024

The following nested code-cell fails execution.

```{code-cell} python
hello = "hello"
there = "there"
phrase = f"{hello}, {there}!"
print(phrase)
```
some markdown code example

:tags: remove-input
print("This will show output with no input!")

> ```{code-cell} python
> :tags: remove-input
> print("This will show output with no input!")
> ```

the above block

This is the AST:

image

The failure:
image

That is the markdown cell that we are trying to execute in python.

The selection here:
https://github.com/executablebooks/mystmd/blob/bug/code-block/packages/myst-execute/src/execute.ts#L282

it does not take into account nested blocks, and/or special blocks that should only contain a code/output pair.
Instead there is a block that contains a code-cell.

The next error is here:
https://github.com/executablebooks/mystmd/blob/bug/code-block/packages/myst-execute/src/execute.ts#L120

Which I think needs an executable flag on it.

Regardless, needs some more test cases on these nested select cases, and/or being more specific about the block we are filtering for.

@rowanc1 rowanc1 added the bug Something isn't working label Apr 18, 2024
@agoose77 agoose77 self-assigned this Apr 18, 2024
@agoose77
Copy link
Collaborator

Ah, it looks like the selector is wrong
block:has(code[executable=true]):has(output),inlineExpression

I hadn't thought about executable nodes being nested inside quotes. I didn't think we wanted to support that kind of usage, but I have not personally given it much thought (and I reckon I'd be in favour of it).

The issue is that our version of unist-util-select (and in turn, unist) doesn't let us tighten the descendent selector in has(child) to `has(> child), which is unfortunate.

I'll give this some cycles on Monday, when I'm back in the office, but anyone else is welcome to tackle it!

@rowanc1
Copy link
Member Author

rowanc1 commented Apr 18, 2024

I think there are some quick tightening we can do, including labelling the block on the way through. The other block is actually a markdown cell, which happens to contain a code-cell. Not allowed in Jupyter Notebooks, but probably fine to allow here, and I kinda like that it should work.

The other selector should at least look for an executable code cell. That would have made it "work" but the one cell would be executed twice.

I think the better plan is to use the data: type here or kind: code-cell to make the selection of blocks much clearer and less based on the structure. I think we talked around parts of this before, but forget what we decided!

@rowanc1
Copy link
Member Author

rowanc1 commented Apr 18, 2024

@agoose77 I have put together a fix in #1136 by changing block.data.type --> block.kind, which is going to specifically select those blocks. It does seem to work and pass various tests, I want @fwkoch's take on this, and I haven't tested with thebe.

I think it is good for the cell kind to be a known attribute of the block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants