fomightez / cairo-jupyter

Jupyter notebooks hosted via mybinder.org where cairo works
https://mybinder.org/v2/gh/fomightez/cairo-jupyter/master?filepath=index.ipynb
3 stars 1 forks source link

CI #5

Open stuaxo opened 6 years ago

stuaxo commented 6 years ago

CI would make make it easier to modify the extension, to start with it could run a notebook that outputs some file and just verify it exists.

fomightez commented 6 years ago

In regards to this line of thought, did you see this https://github.com/binder-examples/continuous-build ?

stuaxo commented 6 years ago

I saw it existed, but didn't have the time to make it work .. are you up for changing the example so it uses our extension ?

fomightez commented 6 years ago

I haven't yet used continuous integration, but I could try.

Are we adding pycairo import to the notebooks though? I was wondering if I should wait until after that change?

stuaxo commented 6 years ago

PyCairo and CairoCFFI should work (I think there might be some sort of issue / not sure if it does work at the moment).

When these notebooks started cairocffi was further ahead, since then pycairo has revived... if it works, we should try defaulting to it.

We should probably have at least one demo that shows cairocffi can work too though.

Up to you about CI, I had a look at the example above and it's more about building docker images to put on Dockerhub, which we probably don't need as we don't have releases yet.

This looks interesting https://github.com/computationalmodelling/nbval it can run the notebook and test if the output matches the cells.

It could be along the lines of getting that working against a test notebook, then seeing if it's possible to do the same in a CI environment.

fomightez commented 6 years ago

I sort of agree that the emphasis in the example is on building, but I think the use of Dockerhub is really just to have a 'central' place that plays well with CircleCI to put the output from Docker2repo. I can tell you from experience testing a lot of these binderized repos that the steps of pushing changes and hitting 'launch' binder to trigger a build gets tedious fast. I think this would be a step forward to eliminate that drudgery and build towards it including tests. In fact, they mention that in a couple of places as the natural next progression, I think.

"We will add templates as they are requested to do additional tasks like test containers, run nbconvert, etc." - Source

That really sounds like what you were looking for when you opened this issue. Granted we need to bring Docker into the mix. Which I am already using for other things.

nbval does look cool and I wonder if that is sort of what they hope to work towards being able to demo. My plan is to get the basic steps they outlined working and then ask them about that test example next.

fomightez commented 6 years ago

The directions worked. I was able to pull the image after pushing a change to the repo without ever dealing with binder site. The only thing I needed to figure out is that I should have used latest to as DOCKER_TAG, but it simply means I need to add the tag to get things to work, such as docker run -it fomightez/cairo-jupyter:8b0bc35c95.

UPDATE: I was successful in getting the tag to update to latest by making a change in repo and pushing. I had changed the environmental variable and tried rerunning the circleci job and that didn't seem to do it. (But maybe I didn't give things enough time to update?)

stuaxo commented 6 years ago

Fantastic :)

Having a quick browse over that circleci file, I think there should be a python-3.6.6 circleci image by now, OTOH - it could be worth waiting till 3.7.0 to upgrade.

Just had a quick look at the jupyter2repo docs and realising that binder is based on it. Not sure how much customisation repo2docker offers, maybe it's possible to have an entrypoint that launches the jupyter... definitely interesting.

I was able to run the docker image locally, will try more in depth next time I get a chunk of time.

fomightez commented 6 years ago

I followed the example here and added a test image creation script. There is a badge to indicate status on the README now. It only does the basics and doesn't deal with the extension yet.

As for integrating the extension, I'll have to even see if I can get something like get_ipython().run_line_magic('reload_ext', 'cairo_jupyter') to work outside of the Jupyter notebook. Maybe I'll just need to call ipython -c in the test instead of python -c? For now it looks like the image the extension makes is just in memory. When you asked about adding CI for helping with development, were you planning to add saving the image to the extension for development phase?

stuaxo commented 6 years ago

A postBuild script might be an option to install and enable the extension.

The image is in memory, but also ends up in the notebook as a png data url - you can see it if you view the source of extension demo ipynb file.

I've had a bit of a play, and think I understand the problem a bit better - by default in ipython in the console display doesn't seem to output png images in the same way as in the browser.

You can pass raw=True and get the data to stdout (looks like the same format that ends up in the notebook file), not sure how useful this is though.

This probably explains why I couldn't get it working in nbval doesn't work for the image data either (though I wonder how hard it would be for it to support?).

At this point unit testing the output methods in the extension might be the quickest route, there are only two methods to test.

display_cairo_surface() display_cairo_context()

I hadn't thought about outputting the images, do you mean regenerating the ones in the notebooks ? It would be nice, though now I've looked the png image support seems a bit lacking when running headlessly.

xvfb with something like phantom or firefox or chrome headless could work, but takes a bit of setting up.

fomightez commented 6 years ago

The postBuild is the suggested way to enable extensions. But I was more focused on downstream of that which it looks like you are looking into. Basically, I was still focused on your suggestion, "CI would make make it easier to modify the extension, to start with it could run a notebook that outputs some file and just verify it exists.". Albeit without running the notebook since the community 'we' (that I know of) haven't worked that out yet. I gather we just need some code that can be run in Python or IPython (presumably) to do more along the lines of what you originally intended in regards to seeing if changes have broken the base pipeline, right? Eventually it would be nice to make it so a notebook (or all) get run and verified code ran without an error.

But I don't think we'll ever be able to check if they are correct automatically, at least not without adding a load of additional dependencies like OpenCV and coding and storing all the images, right? Seems overkill for development.

stuaxo commented 6 years ago

Ah.. yeah, I wrote that before I realised how much hassle it is to run a jupyter notebook headlessly, so I thought we could have a really basic check to see if the file is created or not.

You're right about checking for correctness being overkill ... I have prototyped something like this before, for another project - but don't feel like doing it again here :) there are python libraries that can tell if an image is similar, but we don't really need to check cairo itself.

stuaxo commented 3 years ago

OK, this is interesting nbconvert can run all cells [1]

Github actions seems to have eaten CI on github, we could probably add one that tries running the notebooks to make sure there are no errors at a minimum - we might be able to build something further on that later.

[1] https://stackoverflow.com/questions/35471894/can-i-run-jupyter-notebook-cells-in-commandline

fomightez commented 3 years ago

I've actually played around with using Github Actions for another repo so that adding the nbconvert step would be straight forward. I just need to look into what happens or how to detect the error because I wasn't examining the product of the actions in my other work.

I also wonder if such an action is already shared out there.
(I'm making a note for myself to try and implement this eventually but it won't be any time soon.)