Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for collect and squashoutput #38

Merged
merged 22 commits into from
May 15, 2021
Merged

Tests for collect and squashoutput #38

merged 22 commits into from
May 15, 2021

Conversation

johnomotani
Copy link
Contributor

@johnomotani johnomotani commented Apr 30, 2021

Adds tests for collect() and squashoutput() to the test suite. The tests cover core, SOL, single-null, connected double-null and disconnected double-null topologies (the last two aren't really different because collect() doesn't need the separatrix positions ixseps1 and ixseps2, but is included for completeness - limiter topology is missing, but is not really different from core or SOL for the same reason). The xguards, yguards, tind, xind, yind and zind arguments are all tested, as are the compression and complevel arguments to squashoutput (which verify that we get identical data when using compression). Edit: now added tests for different MXG and MYG values for one case.

If anyone can see anything significant missing, please add it!

@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2021

Codecov Report

Merging #38 (9447a39) into master (4e022e9) will increase coverage by 14.61%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #38       +/-   ##
===========================================
+ Coverage   10.33%   24.94%   +14.61%     
===========================================
  Files          16       16               
  Lines        2312     2385       +73     
  Branches      472      497       +25     
===========================================
+ Hits          239      595      +356     
+ Misses       2063     1708      -355     
- Partials       10       82       +72     
Impacted Files Coverage Δ
boutdata/collect.py 67.44% <50.00%> (+63.66%) ⬆️
boutdata/data.py 41.02% <0.00%> (+4.78%) ⬆️
boutdata/squashoutput.py 38.09% <0.00%> (+38.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e022e9...9447a39. Read the comment docs.

@johnomotani
Copy link
Contributor Author

johnomotani commented Apr 30, 2021

One downside is that the tests are a bit slow (CI takes ~20mins), but I think that's inevitable when reading/writing to/from disk - I think it's more important to test as many arguments to collect and squashoutput as possible to catch edge cases.

@loeiten
Copy link
Member

loeiten commented May 3, 2021

What an awesome piece of work @johnomotani 🥳
I can have a look at it once I get some time at my hands

Copy link
Member

@loeiten loeiten 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 as far as I can see. Again, great work! I only got minor suggestions

boutdata/tests/make_test_data.py Show resolved Hide resolved
boutdata/tests/make_test_data.py Show resolved Hide resolved
boutdata/tests/make_test_data.py Show resolved Hide resolved
boutdata/tests/make_test_data.py Show resolved Hide resolved
boutdata/tests/make_test_data.py Outdated Show resolved Hide resolved
boutdata/tests/test_collect.py Show resolved Hide resolved
boutdata/tests/test_collect.py Show resolved Hide resolved
boutdata/tests/test_collect.py Outdated Show resolved Hide resolved
boutdata/tests/test_collect.py Outdated Show resolved Hide resolved
boutdata/tests/test_collect.py Show resolved Hide resolved
@johnomotani
Copy link
Contributor Author

Thanks @loeiten! I think I've addressed all your comments now.

Copy link
Member

@loeiten loeiten left a comment

Choose a reason for hiding this comment

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

LGTM. Great work 😃

Makes test data creation more compact and easier to read.
@johnomotani johnomotani merged commit a6199d3 into master May 15, 2021
@johnomotani johnomotani deleted the test-collect branch May 15, 2021 12:19
@johnomotani johnomotani mentioned this pull request May 26, 2021
ZedThree pushed a commit that referenced this pull request Apr 8, 2022
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.

3 participants