WIP: Move a Travis job to GitHub Actions #8489

GuillaumeFavelier · 2020-11-05T16:20:51Z

This PR moves the top Travis job (python3.7, conda, ...) to GitHub Actions.

GuillaumeFavelier · 2020-11-05T16:30:59Z

My bad, I just checked and actually the top job uses python 3.8, it's in environment.yml:

dependencies:
- python>=3.8

GuillaumeFavelier · 2020-11-06T08:06:57Z

The great majority of the tests succeed except:

mne/preprocessing/tests/test_fine_cal.py:

____________________________ test_compute_fine_cal _____________________________
mne/preprocessing/tests/test_fine_cal.py:111: in test_compute_fine_cal
    _assert_shielding(raw_sss_py, raw, 59, 60)
E   AssertionError: Shielding factor not 59.000 <= 58.914 < 60.000
E   assert 59 <= 58.914246613141785

mne/preprocessing/tests/test_xdawn.py:

 __________________________ test_xdawn_regularization ___________________________
mne/preprocessing/tests/test_xdawn.py:203: in test_xdawn_regularization
    xd.fit(epochs)
E   Failed: DID NOT RAISE <class 'ValueError'>

GuillaumeFavelier · 2020-11-06T08:15:34Z

I guess I could decrease the tolerance to 58 for test_fine_cal but I am not familiar with Xdawn.

Is it because 42 is evaluated to True in:

    # Init xdawn with bad parameters
    pytest.raises(ValueError, Xdawn, correct_overlap=42)

in Xdawn.__init__():

...
        self.correct_overlap = _check_option('correct_overlap',
                                             correct_overlap,
                                             ['auto', True, False])

Any idea @agramfort, @larsoner ?

GuillaumeFavelier · 2020-11-06T10:06:37Z

I tracked the new Travis failure to #8486:

mne/tests/test_report.py:95: in test_render_report
    report.parse_folder(data_path=tempdir, on_error='raise')
<decorator-gen-342>:24: in parse_folder
    ???
mne/report.py:1474: in parse_folder
    if len(inst.filenames) > 1:
E   AttributeError: 'Evoked' object has no attribute 'filenames'

I lack context to find the optimal fix for this one and I will continue to investigate but maybe you know an obvious fix @agramfort, @hoechenberger ?

hoechenberger · 2020-11-06T10:10:48Z

I lack context to find the optimal fix for this one and I will continue to investigate but maybe you know an obvious fix @agramfort, @hoechenberger ?

Don't waste time on this, I will fix this momentarily, sorry about this

GuillaumeFavelier · 2020-11-06T10:12:41Z

Thanks for the swift answer!

hoechenberger · 2020-11-06T10:31:51Z

@GuillaumeFavelier This should be fixed in #8491, sorry for delaying you here. We somehow overlooked a failing CI before merging yesterday

larsoner · 2020-11-06T13:28:19Z

Any idea @agramfort, @larsoner ?

Sometimes these depend on little numerical differences caused by different linear algebra libraries. I would just make the necessary small adjustments, see for example https://github.com/mne-tools/mne-python/pull/8046/files#diff-713b7a9b5d7636d6c89e985d8fb1bd4c88ca14290509f426f4dffd2fa99305cfR196 and https://github.com/mne-tools/mne-python/pull/8046/files#diff-998593fc4edeccd3a930c2dd734d4a0042cac66a24527f9d069d46afd4f7c338R690 in the conda-forge PR

hoechenberger · 2020-11-06T14:54:15Z

#8491 has been merged, you can rebase on master

larsoner · 2020-11-06T14:55:54Z

If this is set up correctly (meaning: does a build of the a merged version of the PR, and not just the branch itself) then just restarting the action should be enough. I've done that to see if it succeeds

larsoner · 2020-11-06T14:56:17Z

(I couldn't find a way just to restart the failed job, which is not great, so I restarted both)

GuillaumeFavelier · 2020-11-06T15:06:04Z

I couldn't find a way just to restart the failed job

Maybe I should split the linux job in a different file? (I mean creating another workflow)

larsoner · 2020-11-06T15:09:57Z

Maybe I should split the linux job in a different file?

I just noticed that things are almost entirely duplicated in the current file. I'd go the opposite route and try to unify the two approaches and make things more DRY. Then we can figure out how to properly make it so you can restart one job versus another. I'm guessing that it's just another entry in the given file or something. Worth investigating how to get the the GitHub actions interface to support it.

GuillaumeFavelier · 2020-11-06T15:12:38Z

I'd go the opposite route and try to unify the two approaches and make things more DRY

Let's do this then

larsoner · 2020-11-06T15:18:10Z

Looks like it's a requested feature

https://github.bokerqi.topmunity/t/ability-to-rerun-just-a-single-job-in-a-workflow/17234/4

Another option would be to split into two files, but try to reuse code where possible by using tools/some_script.sh. Maybe this would allow us to maintain some amount of DRY code and also get us the ability to restart jobs separately?

GuillaumeFavelier · 2020-11-06T15:33:56Z

I'll try to find the right balance

GuillaumeFavelier · 2020-11-06T16:16:18Z

Only one script to:

Install MNE
Download the testing data
Print the locale
Run the tests

will generate a huge log.

larsoner · 2020-11-06T16:26:14Z

You can split it into four separate scripts, that's still pretty DRY

larsoner · 2020-11-06T17:16:13Z

.github/workflows/linux.yml

+
+jobs:
+  Conda:
+    if: "!contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"


This might be a good time to follow scipy/scipy#12921 and add:

Suggested change

if: "!contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"

if: "github.repository == 'mne-tools/mne-python' && !contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"

larsoner · 2020-11-06T17:17:35Z

.github/workflows/linux.yml

+      - uses: actions/cache@v1
+        env:
+          # Increase this value to reset cache if environment.yml has not changed
+          CACHE_NUMBER: 0
+        with:
+          path: ~/conda_pkgs_dir
+          key: ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{ hashFiles('environment.yml') }}


I don't think we want to do this. I know it speeds things up, but it stops us from continuously checking to make sure that our conda instructions work, which has shown important bugs in the past.

larsoner · 2020-11-06T17:18:10Z

.github/workflows/linux.yml

+          source tools/get_minimal_commands.sh
+          mne_surf2bem --version


These should live in get_minimal_commands.sh

larsoner · 2020-11-06T17:18:40Z

.github/workflows/linux.yml

+          mne_surf2bem --version
+        name: 'Install dependencies'
+      - shell: bash -el {0}
+        run: |


These should all be one-liners so get rid of | to make more compact?

larsoner · 2020-11-09T13:22:06Z

Restarting did not help:

https://github.com/mne-tools/mne-python/pull/8489/checks?check_run_id=1374022972

This suggests that a merged version of the PR is not being built, but rather the plain branch itself. Can you look into it?

GuillaumeFavelier · 2020-11-09T13:41:18Z

Yes, I'm on it

GuillaumeFavelier · 2020-11-09T14:08:18Z

This time actions/checkout merged the right commit:

HEAD is now at 62cc1daaf Merge 73ec4ad3154de29f34cefd37625100fd47347208 into e70247800458ff0a36fbe90b134841087ce95c6e

73ec4ad: latest commit on the branch
e702478: latest commit on master

GuillaumeFavelier · 2020-11-09T14:10:04Z

I will still wait for another commit on master to test that Restart workflow works the same way.

larsoner · 2020-11-09T14:52:05Z

Looks like it worked! Ready for merge from your end @GuillaumeFavelier ?

GuillaumeFavelier · 2020-11-09T14:53:38Z

I am not 100% sure yet. Because I pushed another commit. I would like to wait for another commit after e702478 to test Restart workflow if you don't mind.

larsoner · 2020-11-09T14:56:23Z

Okay I pushed 29ced0f and then restarted the macOS GitHub action

GuillaumeFavelier · 2020-11-09T14:57:08Z

Many thanks :)

GuillaumeFavelier · 2020-11-09T14:58:48Z

So on the restarted job:

HEAD is now at 62cc1daaf Merge 73ec4ad3154de29f34cefd37625100fd47347208 into e70247800458ff0a36fbe90b134841087ce95c6e

It's not the updated master :(

larsoner · 2020-11-09T15:04:23Z

It's not the updated master :(

Okay so maybe we have to just live with GitHub's limitation here. Can you push an empty commit here to see if that one gets the latest master merge? If so it's probably good enough

GuillaumeFavelier · 2020-11-09T15:07:43Z

A commit seems necessary indeed:

HEAD is now at f688b1199 Merge c60c3ad5dcbada887e879ffbb154a1aa2d39caf5 into 29ced0fa95fafa3c81474fe08e249f90563d9fc8

It is up to date now.

larsoner · 2020-11-09T15:17:40Z

Good enough, thanks @GuillaumeFavelier

GuillaumeFavelier added 3 commits November 5, 2020 17:14

TST: Add support for linux

2d68996

Update CONDITION variable

f362e3c

Fix CONDITION variable

44c0dc0

GuillaumeFavelier self-assigned this Nov 5, 2020

Update workflows

eb38823

GuillaumeFavelier added 3 commits November 6, 2020 09:17

TST: Try

5ef9223

TST: Try again

131c040

Merge branch 'master' into enh/gha_linux

19e9a52

GuillaumeFavelier added 2 commits November 6, 2020 14:31

Adjust tests [skip travis][skip circle][skip azp]

dc3b4a0

Adjust tests [skip travis][skip circle][skip azp]

6c7c297

GuillaumeFavelier added 2 commits November 6, 2020 17:05

Split workflows

d2f83a8

Merge branch 'master' into enh/gha_linux

0e47b31

Add comments

02e5f72

Add comments

aa849de

Split main script and migrate MacOS too

5b062d7

larsoner reviewed Nov 6, 2020

View reviewed changes

GuillaumeFavelier added 3 commits November 6, 2020 18:27

Integrate reviews

902efd3

Remove codna caching

04f60cb

Use latest version

1d12085

GuillaumeFavelier changed the title ~~WIP: Move a Travis job to GitHub Actions~~ MRG: Move a Travis job to GitHub Actions Nov 9, 2020

GuillaumeFavelier changed the title ~~MRG: Move a Travis job to GitHub Actions~~ WIP: Move a Travis job to GitHub Actions Nov 9, 2020

TST: Try fetch-depth=0 [skip travis][skip circle][skip azp]

73ec4ad

TST: Trigger with empty commit

c60c3ad

larsoner merged commit 3b909b7 into mne-tools:master Nov 9, 2020

GuillaumeFavelier deleted the enh/gha_linux branch November 9, 2020 15:19

	if: "!contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"
	if: "github.repository == 'mne-tools/mne-python' && !contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"

Uh oh!

WIP: Move a Travis job to GitHub Actions #8489

WIP: Move a Travis job to GitHub Actions #8489

Uh oh!

Conversation

GuillaumeFavelier commented Nov 5, 2020

Uh oh!

GuillaumeFavelier commented Nov 5, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

hoechenberger commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

hoechenberger commented Nov 6, 2020

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

hoechenberger commented Nov 6, 2020

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

GuillaumeFavelier commented Nov 6, 2020

Uh oh!

larsoner commented Nov 6, 2020

Uh oh!

larsoner Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

larsoner Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

larsoner Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

larsoner Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

larsoner commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

larsoner commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

larsoner commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

larsoner commented Nov 9, 2020

Uh oh!

GuillaumeFavelier commented Nov 9, 2020

Uh oh!

larsoner commented Nov 9, 2020

GuillaumeFavelier commented Nov 6, 2020 •

edited

Loading

GuillaumeFavelier commented Nov 9, 2020 •

edited

Loading