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

Test notebooks locally #137

Merged
merged 2 commits into from
Dec 9, 2021
Merged

Test notebooks locally #137

merged 2 commits into from
Dec 9, 2021

Conversation

nhuet
Copy link
Contributor

@nhuet nhuet commented Dec 6, 2021

  • Add nbmake in dev dependencies for testing notebooks locally with pytest
  • Add explanation in contributing guide.

@nhuet
Copy link
Contributor Author

nhuet commented Dec 6, 2021

To test the PR, you have to install again with poetry (i do not know how to update the poetry install cleanly). And then type
poetry run pytest --nbmake notebooks -v

@nhuet
Copy link
Contributor Author

nhuet commented Dec 6, 2021

I see that the workflow crashed but is again a seg fault for tests/solvers/cpp that happens from time to time with ubuntu or macos runners (here ubuntu). It would be nice to find out why this is happening. And if someone with the proper rights (@galleon ?) can relaunch the workflow, it should work normally... (it worked on my branch that this PR is pulling: https://github.com/nhuet/scikit-decide/actions/runs/1544104432)

@nhuet
Copy link
Contributor Author

nhuet commented Dec 9, 2021

Thanks @galleon ! So now that the workflow passed, what is missing for the merge ?

@galleon
Copy link
Contributor

galleon commented Dec 9, 2021

Still need to move nbmake as a dev dependency.

@nhuet
Copy link
Contributor Author

nhuet commented Dec 9, 2021

Weird I was sure to have added it to dev section... I move it in pyproject.toml then reinstall everything (:cry:) so that poetry.lock update accordingly and i repush...

@nhuet nhuet force-pushed the nh/test_notebooks branch from e77d3a9 to ef86925 Compare December 9, 2021 14:48
@nhuet nhuet force-pushed the nh/test_notebooks branch from ef86925 to 7e74454 Compare December 9, 2021 14:50
@nhuet
Copy link
Contributor Author

nhuet commented Dec 9, 2021

I am not sure if I am doing it properly. To add a dependency while having a poetry.lock, I

  • added nbmake to dev dependencies in pyproject.toml
  • run a poetry update so that the new dependencies are taken into account (else the poetry install fails)

=> it results in all versions of dependencies being recomputed and updated to newest available (not necessarily what we wanted). Maybe I could have first done a poetry update in a separate commit, before adding the new dep nbmake...

Anyway, it should work like that, let us see what the workflow give us after this update...

The poetry.lock file result from
- having modified pyproject.toml to add nbmake as dev dependency
- `poetry update`
@nhuet nhuet force-pushed the nh/test_notebooks branch from 7e74454 to e1ebda8 Compare December 9, 2021 14:52
@galleon galleon merged commit 7b6c600 into airbus:master Dec 9, 2021
@nhuet nhuet deleted the nh/test_notebooks branch December 13, 2021 10:43
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