welfare-state-analytics / riksdagen-corpus

Swedish parliamentary proceedings - Riksdagens protokoll 1867-today
Other
26 stars 5 forks source link

Update example notebook #318

Closed ninpnin closed 1 year ago

ninpnin commented 1 year ago

Copied from https://colab.research.google.com/drive/1C3e2gwi9z83ikXbYXNPfB6RF7spTgzxA?usp=sharing#scrollTo=1qaDk9uoCw_i which is currently only available via a link in the README

MansMeg commented 1 year ago

Is it hard to test the notebook automatically with a test?

ninpnin commented 1 year ago

This seems promising https://github.com/treebeardtech/nbmake , but it takes a little while to see if it actually works, cause we need to run it here along with all other tests.

MansMeg commented 1 year ago

Great. Lets see how it works. Yes, it would be good to run this in a separate workflow. Ie. if it gets defunct, it should not hinder other tests to run.

ninpnin commented 1 year ago

I added it in the same workflow. It's the last one to be run though, so it shouldn't interfere with other workflows.

And it seems to work which is good.

MansMeg commented 1 year ago

Please put it as a separate workflow since it build upon another codebase.

If the code fails (due to nbmake) in 4 years and nobody knows nbmake in the project, we dont want to hold up revisions to the corpus. Its a maintenance issue of the corpus.

ninpnin commented 1 year ago

It's just a python module?

MansMeg commented 1 year ago

Ok. What is your guess will it work and be maintained the next 5 years? I can judge that for R packages, but not Python libs.

MansMeg commented 1 year ago

Ok. What is your guess will it work and be maintained the next 5 years? I can judge that for R packages, but not Python libs.

ninpnin commented 1 year ago

I separated them into two 'builds' now, as can be seen above in the pull request. The notebooks part is siloed into its own machine now, and can be removed if it becomes an issue in the future.

MansMeg commented 1 year ago

Great!

MansMeg commented 1 year ago

@BobBorges Could you do a wuick review on the new notebook? Then I think we can merge.

BobBorges commented 1 year ago

How quick? :D JK, yes, I will look at it in the evening -- I'm currently on the move.

MansMeg commented 1 year ago

No problem, whenever you have time.

BobBorges commented 1 year ago

It looks reasonable.

Do we know all the code runs? I didn't test it, but it looks like it should work.

ninpnin commented 1 year ago

@BobBorges the new module test the notebooks. Here's the log from the run:

============================= test session starts ==============================
platform linux -- Python 3.8.16, pytest-7.3.2, pluggy-1.0.0
rootdir: /home/runner/work/riksdagen-corpus/riksdagen-corpus
plugins: nbmake-1.4.1
collected 1 item

examples/corpus-walkthrough.ipynb .                                      [100%]

========================= 1 passed in 76.57s (0:01:16) =========================