Closed johnomotani closed 4 years ago
Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
There are lots of PEP8 complaints from xbout/tests/test_boutdataarray.py
about too-long lines, but I think it's best to ignore them. Those lines are effectively a table of expected results, which are much easier to read without splitting the lines.
Might be a way to comment the region to say "ignore this bit"?
Might be a way to comment the region to say "ignore this bit"?
It's possible, but as far as I can see (https://flake8.pycqa.org/en/latest/user/violations.html) requires a comment on each line. That's not very nice, but I guess is better in the long run as it means we can run flake8 over the whole project and not get errors that we want to ignore. I'll add the suppressions.
:+1: suppressing the errors that we want to ignore showed a too-long line error that needed fixing but which I had missed in the noise before, so I think it is a good idea. Thanks @ZedThree!
Merging #105 into master will increase coverage by
1.94%
. The diff coverage is79.48%
.
@@ Coverage Diff @@
## master #105 +/- ##
==========================================
+ Coverage 48.05% 50.00% +1.94%
==========================================
Files 11 11
Lines 1105 1172 +67
Branches 227 233 +6
==========================================
+ Hits 531 586 +55
- Misses 506 514 +8
- Partials 68 72 +4
Impacted Files | Coverage Δ | |
---|---|---|
xbout/geometries.py | 75.25% <70.00%> (+0.81%) |
:arrow_up: |
xbout/boutdataset.py | 66.86% <80.76%> (+1.92%) |
:arrow_up: |
xbout/boutdataarray.py | 70.83% <80.95%> (+7.87%) |
:arrow_up: |
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 41921a6...5cf6d0c. Read the comment docs.
I feel like there's probably a more compact way to write the tests, but these are nice and explicit at least!
Includes changes in #104, which ideally should be merged before reviewing this.
Thanks to @friva000 from whose routines I lifted the fft interpolation logic.