Skip to content

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Nov 4, 2021

  1. Don't install upstream/main for now, use stable
  2. Use upstream/main rather than marsipu/pg_test

@larsoner
Copy link
Member Author

larsoner commented Nov 4, 2021

@marsipu this:

  1. implements the os/mne matrix, plus an old/compat 3.7 version and no opengl variants -- so 9 variants (bad) but each is only ~5 min (good) so overall a good balance of coverage versus CI burden
  2. injects an ignore for the warning about not having OpenGL (hopefully temporary and we'll go with opt-in so this warning goes away)
  3. revamps the tests and dev instructions so that things are simpler
  4. runs the benchmarks on CIs in addition to mne-python unit tests

With this I think we can ensure that things work. Ideally we would have a nightly scheduled job that emailed us on failure. One option is to switch to Azure Pipelines, because I know how to configure such nightly emails there. It looks like there might not be a good solution for GitHub Actions currently. But in the meantime I think this PR is a step in the right direction, so feel free to merge if you're happy!

@larsoner
Copy link
Member Author

larsoner commented Nov 4, 2021

@marsipu I'm going to merge this so that the opt-in behavior can be tested properly, but feel free to comment and I'm happy to open another PR.

@larsoner larsoner merged commit c4d1968 into mne-tools:main Nov 4, 2021
@larsoner larsoner deleted the tools branch November 4, 2021 14:30
@marsipu
Copy link
Member

marsipu commented Nov 4, 2021

Thanks a lot @larsoner for taking care of the test-matrix and the improved benchmarks. I really like the improved cli-interface and the presentation of the results.

I am a bit confused by the results of defaults, since they should be equal to the results of use_opengl=False now, right?

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