nbPython

[HugoGranstrom]

implements #82

I haven't tested this extensively yet but the non-string variant seems to work. I have one problem though, I can't have two overloads with the same name. Because to find out if it is a string it has to typecheck the code, the python code... So we need to use two separate names for them. Either one of them is nbPython and the other is nbPythonCode or nbPythonString. Or both have the more verbose names. Which of them do you prefer?

TODO:

• [x] Get string version working
• [ ] Write tests

[Vindaar]

Probably not possible for some reason I'm not aware of, but the python string case is for string literals, no?

Why not turn it into a macro and dispatch based on the node kind? If the argument is a string literal use that, otherwise treat is as code?

I.e. something like this in properly done:

import macros

proc doStrStuff(n: NimNode): NimNode = discard

proc doAstCodeStuff(n: NimNode): NimNode = discard

macro nbPython(code: untyped): untyped =
  echo code.treerepr
  if code.len == 1 and code[0].kind in {nnkStrLit, nnkTripleStrLit}:
    result = doStrStuff(code[0])
  else:
    result = doAstCodeStuff(code)

nbPython:
  a = 5
  print(a + "5")

nbPython: """
  b = 5
  print(b + "123")
"""

[pietroppeter]

we need to use two separate names for them

although @Vindaar 's solution seems elegant I would keep it simple and use nbPython for the string based variant (main api) and use nbPythonim for the code variant. Indeed it is a weird variant that takes nim code and interprets it as Python code (and we might expect not to support full python).

[pietroppeter]

Probably not possible for some reason I'm not aware of, but the python string case is for string literals, no?

actually thinking about this made me realize that having a template that takes a string as input (or in this case an untyped body expression that evaluates as string) make possible to generate more complex python code than what would be possible with the string literal approach.

[HugoGranstrom]

I think I'll agree with Pietro on this one, allowing the input to be a string of any kind, literal or not, is more powerful. I'm not really sure why you would want to generate python code, but I'm sure that someone will find a cool way of using it.

I'm not really a fan of the name nbPythonim though, the -nim ending doesn't make sense to me from a user perspective. They won't be thinking "Oh I'll write my Python code and then it will be interpreted as untyped Nim code". They will more think "Oh, I'll just write my Python code in this code block". So I'd prefer something like nbPythonCode or nbPythonCodeBlock which is more descriptive of what it does.

[pietroppeter]

ok then let's go with nbPythonCode, agree in general more descriptive is better

[HugoGranstrom]

Are you okay with me adding tests for this as well? (And making Python a test dependency practically) And adding nimpy as a dependency for nimib?

[HugoGranstrom]

Here's a sneak peek of how the two variants looks when highlighted:

[pietroppeter]

while we are talking about highlighting, a though on the api for that. one of the reason that nimib uses often the nb prefix is that I did not want to "pollute" the namespace with common identifiers that someone would want to use (especially since one of the original though was a DSL that you could drop in an existing file without changing anything else). Something that slightly bothered me was that nimib / boost diverges a bit from that (md might have been something that one would want to use to convert a markdown string to html) but it was very limited to only two identifiers. Now we plan to introduce a new identifier in nimib / boost called python... On the other hand I see how nice can be the highlighting and I am actually realizing that people might want it in general in nim for html or other stuff (also in nimib codebase it could be used)... so what if we change the api of nimib / boost to be hlMd, hlFmd, hlPy, hlHtml, hlJs, ...) or something similar (maybe the F should go at the end and every variant should have a F variant...)? it might also help making it clearer that it serves the purpose of highlighting... it would be of course a breaking change. and probably we should open a separate issue for a discusion about this..

[HugoGranstrom]

That's very true, thought about that as well. python is a really bad name to call it. The hlLang(F) syntax is fine by me 👍 I could implement it here and now if needed so no need for a separate issue unless we suspect someone else to have opinions on it. Breaking changes are fine as long as they have a purpose, which this definitely has. Adding depreciation to md and fmd and removing them in v0.4 sounds like a solid plan to me.

[pietroppeter]

Are you okay with me adding tests for this as well? (And making Python a test dependency practically) And adding nimpy as a dependency for nimib?

ah, righht! nimpy as a dependency of nimib I would prefer to avoid. is there a way to hide this kind of functionality so that it is not imported if the library is not available (a static check on import error, maybe even a when compiles(import nimpy) would be enough)? I think in the future something like that would be useful also for other stuff (e.g. something that shows a html table starting from a datamancer dataframe would need datamancer...). If we manage to do that we could test it in the same way (and in the CI it is fine to add python and nimpy as dependency, we already have ggplotnim and others...).

the other option would be to package this kind of functionalities as separate libraries but I guess if it can be avoided it would be better for now.

[HugoGranstrom]

Yep it's possible to give an error if it can't find nimpy: https://forum.nim-lang.org/t/3752 I'll add it 👍

the other option would be to package this kind of functionalities as separate libraries but I guess if it can be avoided it would be better for now.

I thought about doing that at first, but then it turned out to be so little code so it didn't feel worth making it a separate package.

[HugoGranstrom]

We have a problem for the non-string version. List comprehensions don't work:

nbPythonCode:
    s = [i for i in range(3)]

Gives error:

Error: expected: ']', but got: 'keyword for'

So it seems we've managed to go beyond the capabilities of the Nim Compiler 🙃 Do we still want to keep this or are we satisfied with the highlighting of strings using hlPy?

[pietroppeter]

Do we still want to keep this or are we satisfied with the highlighting of strings using hlPy?

well, it is true that since as demonstrated it breaks so easily, it could be removed and added to some example doc as a DIY novelty element, but since it is already there let's keep it and maybe we will move it out later; it should be anyway made clear that is not something that we plan to support. if you feel you want to remove it go ahead.

[HugoGranstrom]

well, it is true that since as demonstrated it breaks so easily, it could be removed and added to some example doc as a DIY novelty element, but since it is already there let's keep it and maybe we will move it out later; it should be anyway made clear that is not something that we plan to support. if you feel you want to remove it go ahead.

I don't feel happy about making it available to users if we won't support it. But if you want to keep it for the future for a curiosity post we could keep it but not export it. How does that sound?

[HugoGranstrom]

Btw, the CI seems to choke on the nbPython test. It doesn't seem to execute the code correctly or something like that as nb.blk.output is empty 🤔

[Suite] nbPython
[nimib] 11 nbPython: 
    /home/runner/work/nimib/nimib/tests/tnimib.nim(99, 26): Check failed: nb.blk.output == "[0, 2, 4]\n3.14\n"
    nb.blk.output was 
  [FAILED] nbPython string

Could you test locally on your machine and see if it works? It works for me under WSL but I haven't got nimpy to work properly on my Windows install.

[pietroppeter]

I don't feel happy about making it available to users if we won't support it. But if you want to keep it for the future for a curiosity post we could keep it but not export it.

probably better to just remove it. We know where to find it if we need. I just realized that also the "untyped" version of nbFile was not working and will be removed (see latest commit in #81 ).

Could you test locally on your machine and see if it works?

well if it is only the CI throwing problems we can skip the test and open an issue to fix the CI for that later.

I also just realized that this is a PR against my release-0.3 branch. I actually plan to merge that branch since I want to try and use it in my nimibook and getting-started PRs (changing dependency to nimib@#head) to see if everything works. Then when everything works I will open a new PR with a new branch to finalize the release (and later will merge in the PRs for nimibook and getting-started with the correct dependency). In the meantime the nbPython could be merged on main independently.

[HugoGranstrom]

well if it is only the CI throwing problems we can skip the test and open an issue to fix the CI for that later.

Yeah as long as it works on actual machines that's fine. I'll boot up my old laptop with linux and double-check that it works on linux there as well.

I also just realized that this is a PR against my release-0.3 branch. I actually plan to merge that branch since I want to try and use it in my nimibook and getting-started PRs (changing dependency to nimib@#head) to see if everything works. Then when everything works I will open a new PR with a new branch to finalize the release (and later will merge in the PRs for nimibook and getting-started with the correct dependency). In the meantime the nbPython could be merged on main independently.

The target branch is changed. I'll modify the code to make use of the slim code block and resolve the conflicts today or tomorrow.

[HugoGranstrom]

Regarding the CI, look at the nimpy page in getting-started: https://scinim.github.io/getting-started/external_language_integration/nim_with_py.html

Notice how the code discard py.print("Hello world from Python..") doesn't produce an output. It's basically the same problem as we have here. I've tried it on my linux laptop now and it works if you compile it from the command line. But if I try to compile it with nimiBoost it doesn't show the output at all. So I suspect that it has something to do with the shell it is executed in and whether nimpy writes to the same stdout as nimib is capturing from or something along those lines. In other words, the shell that VSCode creates to run the build command is somehow different from the ordinary terminal shell. And if that is the case I have no idea how to solve that. It basically makes all automatic building systems using CI not able to use nbPython properly.

[HugoGranstrom]

I'd say this is ready for merging now that the nbPython test is disabled and the CI seems happy enough.

[pietroppeter]

Thanks for this! Yes, the CI issue is likely due to some weird shell thing that does not allow to capture output correctly.