Repository navigation
Cleanup skipped and xfailed unittests - #3008
erikvansebille merged 20 commits into
Conversation
erikvansebille
changed the title
As I think that duplicate axes are now supported by model.py
| @pytest.mark.skip(reason="Pending ParticleFile refactor; see issue #2386") | ||
| @pytest.mark.parametrize("dt", [-np.timedelta64(1, "s"), np.timedelta64(1, "s")]) | ||
| @pytest.mark.parametrize("maxvar", [2, 4, 10]) | ||
| def test_pset_repeated_release_delayed_adding_deleting(fieldset, tmp_parquet, dt, maxvar): |
There was a problem hiding this comment.
I spent quite some time on making this test work for v4, but then decided that it was too complex because the parquet format and the kernel loop have changed so much. So I suggest to remove it - we test/check output in tutorials and on other places
| @pytest.mark.skip( | ||
| "Needs updating after refactoring from https://github.com/Parcels-code/Parcels/pull/2646" | ||
| ) # TODO: I think we can just rely on the SGRID metadata for this (which already has robust error messaging). How should discrepencies between SGRID and axis attr be handled? | ||
| def test_dim_with_duplicate_axis(): |
There was a problem hiding this comment.
This test now seems to pass. Parcels doesn't throw an error when a Variable has two dimensions with the same axis. Is that expected?
There was a problem hiding this comment.
Hmmm. Its not expected. Perhaps this should be picked up during a metadata validation step by cross-referencing with SGRID. At the moment SGRID is used as the source of truth, potentially ignoring axis metadata attrs in some cases
There was a problem hiding this comment.
Fixed in a76f618. it turned out to be a commented-out assert-function. Could you quickly check if you agree with this solution?
Since it failed on min-versions test
VeckoTheGecko
left a comment
VeckoTheGecko
left a comment
There was a problem hiding this comment.
Looks good. A question, but no blockers
| @pytest.mark.skip( | ||
| "Needs updating after refactoring from https://github.com/Parcels-code/Parcels/pull/2646" | ||
| ) # TODO: I think we can just rely on the SGRID metadata for this (which already has robust error messaging). How should discrepencies between SGRID and axis attr be handled? | ||
| def test_dim_with_duplicate_axis(): |
There was a problem hiding this comment.
Hmmm. Its not expected. Perhaps this should be picked up during a metadata validation step by cross-referencing with SGRID. At the moment SGRID is used as the source of truth, potentially ignoring axis metadata attrs in some cases
And fixing _core under the hood
Description
This PR cleans up some of the (failing/skipped) unit tests - thereby also making some small fixes to
src. Somewhat tangentially, it also renames the .localize function to .localise, as we (try to) adhere to UK spelling in Parcels.Checklist
mainfor normal development,v3-supportfor v3 support)AI Disclosure
None