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

Add support for rich display of Python objects in ScalaPy (rebased) #854

Merged

Conversation

alexarchambault
Copy link
Member

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@alexarchambault
Copy link
Member Author

I don't if that'll fix the issue seen in https://github.com/almond-sh/almond/pull/843…

cc @kiendang

@kiendang
Copy link
Collaborator

kiendang commented Aug 1, 2021

Looks like the error was caught by JNA somehow not being able to load libpython. I did check and libpython was there, handled correctly by python-native-libs. I used papermill to tee notebook cell outputs to stdout (can't do that with nbconvert)

624c197

@kiendang
Copy link
Collaborator

kiendang commented Aug 5, 2021

@alexarchambault Could help me squash bf7c893 and b75f6fa in #843 into "Add rich display for python objects in ScalaPy" as well? Or give me write to the branch I can do it myself.

@alexarchambault
Copy link
Member Author

@kiendang Not sure I see what you mean, but I gave you write access to the repo just-in-case (this should allow you to re-run CI jobs for example).

@kiendang
Copy link
Collaborator

kiendang commented Aug 6, 2021

Ah this PR is from your fork which I don't have access to so cannot push change. Anw thanks!

@kiendang
Copy link
Collaborator

Was supposed to rebase the last two commits but I don't want to mess up your branch while you're still working on it. So we'll just rebase them when the PR's ready to merge.

@kiendang
Copy link
Collaborator

I still have no idea what actually causes the problem, but in case you're looking into it later, you can use jupyter console to run the example interactively and get the error.

JUPYTER_PATH=... jupyter console --kernel ...

The example works fine when you run it interactively in a notebook, also works fine with jupyter console, nbconvert in macOS. The problem seems to only exist in Linux (I tried both Ubuntu and Arch).

@alexarchambault alexarchambault force-pushed the scalapy-rich-print-rebased branch from ec05df8 to 706a4f0 Compare August 18, 2022 14:14
@alexarchambault
Copy link
Member Author

alexarchambault commented Aug 18, 2022

I was able to run the example notebook manually in a Jupyter console fine on my Ubuntu machine (by running ./mill -i -w jupyterConsoleFast from commit 0d9607c). I was using Java 17, trying again with Java 8 right now... Edit: seems to work fine from Java 8 too.

@kiendang
Copy link
Collaborator

Hmm so still not sure why it fails while running non-interactively. Maybe try with ScalaPy 0.5.2 instead of 0.5.0 too? But I remember trying that locally a while back and still got the error.

@alexarchambault
Copy link
Member Author

I think I found the issue. It originates from some JNA version mismatch. Ammonite and scala-compiler were pulling JNA 5.9.0 (see cs resolve almond:0.13.1 --what-depends-on net.java.dev.jna:jna), while ScalaPy needs a newer version (see cs resolve me.shadaj::scalapy-core:0.5.2). The underlying JNA error was this one.

Merging with main once #974 is merged should fix things (maybe with a few version bumps too).

Not sure why it only happens in non-interactive mode... Maybe Ammonite or scala-compiler were able to initialize JNA stuff in interactive mode (without a version mismatch for them), and then ScalaPy was using an already initialized JNA, so it wasn't having issues.

@alexarchambault
Copy link
Member Author

scalapy/scalapy#294 would have helped debug this too I think (by not trapping the underlying JNA error).

@kiendang kiendang force-pushed the scalapy-rich-print-rebased branch from 5a17fc1 to 73db690 Compare August 20, 2022 12:56
@kiendang
Copy link
Collaborator

🎉🎉🎉

@alexarchambault
Copy link
Member Author

Merging!

@alexarchambault alexarchambault merged commit 348f654 into almond-sh:main Aug 22, 2022
@alexarchambault alexarchambault deleted the scalapy-rich-print-rebased branch August 22, 2022 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants