Skip to content

Conversation

@maxrjones
Copy link
Member

@maxrjones maxrjones commented Nov 16, 2022

Description of proposed changes

This PR improves the xbatcher test suite through the following changes:

  • Use xarray testing methods on batches for more informative diffs
  • Add utility functions for testing the dimensions and shape of batches and the batch generator length. These add a proportionally large amount of code, but are helpful for catching inconsistencies and preventing regressions.
  • Add a test for the batch_dims parameter. As expected, the test currently fails due to Batch generation with batch_dims in v0.2.0 is about 10-20times slower #121
  • Add docstrings to the tests

To-do:

  • Open a few issues based on some inconsistencies I noticed when working on this PR

Addresses #83

@maxrjones maxrjones added the maintenance Maintenance tasks label Nov 16, 2022
@maxrjones maxrjones changed the title WIP: Improve tests Improve tests Nov 18, 2022
@maxrjones maxrjones requested a review from jhamman November 18, 2022 01:09
@maxrjones
Copy link
Member Author

Noting that the failing tests (detailed below) are expected due to the bug reported in #121. My opinion is that it is best to merge this PR with the failing tests and fix the bug in a separate PR.

FAILED xbatcher/tests/test_generators.py::test_batch_3d_1d_input_batch_dims[True] - AssertionError: {'sample': 200, 'x_input': 5, 'y_input': 10} != {'y_input': 10, 'x_input': 5, 'sample': 1000}
- {'sample': 200, 'x_input': 5, 'y_input': 10}
?            ^

+ {'sample': 1000, 'x_input': 5, 'y_input': 10}
?            ^^
 : Dimension names and/or lengths differ
FAILED xbatcher/tests/test_generators.py::test_batch_3d_1d_input_batch_dims[False] - AssertionError: {'time': 2, 'x': 5, 'y': 10} != {'time': 10, 'y': 10, 'x': 5}
- {'time': 2, 'x': 5, 'y': 10}
?          ^

+ {'time': 10, 'x': 5, 'y': 10}

@jhamman would you be able to provide a review for this PR?

@maxrjones maxrjones merged commit 3199feb into main Nov 30, 2022
@maxrjones maxrjones deleted the test-update branch November 30, 2022 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants