Skip to content

Avoid stale cache for multimodel statistics regression tests#1030

Merged
bouweandela merged 1 commit intomasterfrom
fix-regression-multimodel-tests
Mar 2, 2021
Merged

Avoid stale cache for multimodel statistics regression tests#1030
bouweandela merged 1 commit intomasterfrom
fix-regression-multimodel-tests

Conversation

@bouweandela
Copy link
Member

@bouweandela bouweandela commented Mar 2, 2021

Description

To show that this actually solves the problem, I

  1. created a branch called fix-regression-multimodel-tests off master before Use nearest scheme to avoid interpolation errors with masked data in regression test #1021 was merged and pushed it, to run the tests and create the pytest cache.
  2. pulled the current master into fix-regression-multimodel-tests and pushed that, to show that the tests fail because of the stale cache.
  3. changed the cache key, committed and pushed to run the tests again and to show that this solves the problem.

You can view the results of the test runs here:
https://app.circleci.com/pipelines/github/ESMValGroup/ESMValCore?branch=fix-regression-multimodel-tests

I added some comments with instructions to change the cache key every time the cached input data is changed, to hopefully avoid this problem in the future.


Before you get started

Checklist


To help with the number pull requests:

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cheers @bouweandela - this was bothering me but I was under the assumption that'd be fixed by the periodic regeneration of the test container...until two days passed and the fail was still there 😁

@bouweandela bouweandela changed the title Avoid stale cache for multimodel statstistics regression tests Avoid stale cache for multimodel statistics regression tests Mar 2, 2021
Copy link
Contributor

@stefsmeets stefsmeets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

If this doesn't avoid problems with unpickling the cached data,
manually clean the pytest cache with the command `pytest --cache-
clear`.
manually clean the pytest cache with the command `pytest --cache-clear`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a side comment, was this formatted by yapf? On my end it prefers the left side 🤔

Copy link
Member Author

@bouweandela bouweandela Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was formatted by me, yapf doesn't modify any strings (or any other part of the code). I feel that is more readable without the line-break in the middle of the argument.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but our autoformatters (if not yapf, then maybe docformatter) will prefer the left version so I gave up on changing it back 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally see those more as useful tools to quickly arrive at a decent formatting than the definitive guide on how stuff should be formatted. Hopefully a newer version of docformatter will be smarter.

@bouweandela bouweandela merged commit 0c79861 into master Mar 2, 2021
@bouweandela bouweandela deleted the fix-regression-multimodel-tests branch March 2, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multimodel statistics regression tests using sample data fail

3 participants