Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

Checks that the CIs are green on Azure for the notebook 3d backend.

@GuillaumeFavelier GuillaumeFavelier self-assigned this Jun 30, 2020
@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Jun 30, 2020

Now there is a warning on Azure for notebook testing:

FutureWarning: Method cleanup(connection_file=True) is deprecated, use cleanup_resources(restart=False).

@GuillaumeFavelier
Copy link
Contributor Author

This is probably because of the new version of jupyter_client:

image

@GuillaumeFavelier
Copy link
Contributor Author

Bumping nbconvert solved the warning but now pytest-notebook is not ready for the change:

AttributeError: 'ExecuteCoveragePreprocessor' object has no attribute 'setup_preprocessor'

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Jun 30, 2020

I think it's fine since nbconvert 6.0.0 is not officially released yet. We could probably ignore for now and report the change on pytest-notebook ?

@GuillaumeFavelier GuillaumeFavelier changed the title TST: Check notebook 3d backend on Azure MNT: Check notebook 3d backend on Azure Jun 30, 2020
@GuillaumeFavelier
Copy link
Contributor Author

This is trickier than it seems. There is an API change in nbconvert 6.0.0 leading to some part of the code to be split in nbclient.

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Jun 30, 2020

As we do not gather coverage informations from this notebook testing, I could consider migrating to nbclient instead.

@GuillaumeFavelier GuillaumeFavelier changed the title MNT: Check notebook 3d backend on Azure MNT: Migrate to nbclient for notebook testing Jun 30, 2020
@GuillaumeFavelier GuillaumeFavelier marked this pull request as ready for review June 30, 2020 11:55
@GuillaumeFavelier
Copy link
Contributor Author

I reported the FutureWarning in jupyter/nbclient#89.

For now, we can just pin jupyter_client<6.1.5.

@GuillaumeFavelier GuillaumeFavelier changed the title MNT: Migrate to nbclient for notebook testing MRG, MNT: Migrate to nbclient for notebook testing Jun 30, 2020
@GuillaumeFavelier
Copy link
Contributor Author

What do you think @agramfort, @larsoner ?

@agramfort
Copy link
Member

I am confused. Why removing pytest-notebook?

@GuillaumeFavelier
Copy link
Contributor Author

We don't use the coverage information and it's not ready yet for nbconvert 6.0.0.

@larsoner
Copy link
Member

What do you mean we don't use it? It should be reported like all other runs (other than style)

@GuillaumeFavelier
Copy link
Contributor Author

It should be reported like all other runs

I was not aware of that

@agramfort
Copy link
Member

agramfort commented Jun 30, 2020 via email

@GuillaumeFavelier
Copy link
Contributor Author

nbclient executes the notebook test.ipynb in the test_notebook_3d_backend() function of the test_notebook.py file.

I'm now completely lost for coverage.

@agramfort
Copy link
Member

agramfort commented Jun 30, 2020 via email

@larsoner
Copy link
Member

nbclient executes the notebook test.ipynb in the test_notebook_3d_backend() function of the test_notebook.py file.

I'm now completely lost for coverage.

As long as it does this in the same Python process, I think coverage should work. And based on the coverage numbers for the notebook backend, it should be working.

https://codecov.io/gh/mne-tools/mne-python/src/master/mne/viz/backends/_notebook.py

@larsoner
Copy link
Member

Closing in favor of #7943

@larsoner larsoner closed this Jun 30, 2020
@GuillaumeFavelier GuillaumeFavelier deleted the check_notebook_backend branch November 15, 2021 10:02
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.

3 participants