Skip to content

Update example notebook #121

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

Merged
merged 2 commits into from
May 24, 2024

Conversation

anutosh491
Copy link
Collaborator

As we plan to deprecate xeus-cling once we release 1.0.0, I think it makes sense to update our example notebook with most of what xeus-cling's example notebook supports.

I think we have most of what is required except Magics and Xtensor support.

@codecov-commenter
Copy link

codecov-commenter commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.08%. Comparing base (c7a3175) to head (1f9a47d).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #121   +/-   ##
=======================================
  Coverage   75.08%   75.08%           
=======================================
  Files          17       17           
  Lines         602      602           
  Branches       59       59           
=======================================
  Hits          452      452           
  Misses        150      150           

@vgvassilev
Copy link
Contributor

@anutosh491, @mcbarton, @tharun571, is there a way to run these demos as part of the tests? This way we are guaranteed they will work.

@mcbarton
Copy link
Collaborator

mcbarton commented May 23, 2024

@anutosh491, @mcbarton, @tharun571, is there a way to run these demos as part of the tests? This way we are guaranteed they will work.

Some parts of it are already being run as part of the tests in Jupyter Kernel Test, so should be possible. It won't work on Windows until the reason why XCppTests fails has been determined.

@anutosh491
Copy link
Collaborator Author

anutosh491 commented May 23, 2024

Some parts of it are already being run as part of the tests in Jupyter Kernel Test, so should be possible

Yes I haven't used the jupyter-kernel-test library personally (https://github.com/jupyter/jupyter_kernel_test) but I guess that is used to test out kernels. As Matthew says the XCppTests do cover some features like displaying the image etc

@vgvassilev
Copy link
Contributor

In my very naive imagination we can/should have some binary that takes a notebook as parameter and returns some jsons for the results... If we don't have such a thing probably that's easy to implement. I think such a thing is critical to make sure xeus-cpp works consistently over time...

@anutosh491
Copy link
Collaborator Author

Yeah that makes sense. I have the following thoughts

  1. Let me open an issue to indicate introducing of a testing framework as the one you're talking about (takes a notebook as input and maybe returns some json as result)
  2. That being said we might not want to block this PR as we didn't have most features like the documentation lookup through the inspect request etc when the notebook was added by Sylvain last year. I like the idea of the general audience seeing our updated notebook.
  3. We might want to look at the testing framework not from a xeus-cpp level but from the xeus-stack (including all kernels). Just like the jupyter kernel test can test all kernels, the binary we're talking about might cover all of xeus-stack.

cc @vgvassilev @mcbarton if you're thoughts match with mine, you can maybe review and merge and I'll address the 1st point then.

@vgvassilev
Copy link
Contributor

Ok, let’s open an issue for this and move forward with the pr.

@anutosh491
Copy link
Collaborator Author

Raised an issue here ##122

@anutosh491 anutosh491 requested a review from vgvassilev May 24, 2024 07:59
@vgvassilev vgvassilev merged commit e4a86ec into compiler-research:main May 24, 2024
8 checks passed
@anutosh491 anutosh491 deleted the update_notebook branch May 24, 2024 11:05
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.

4 participants