boutproject / xBOUT

Collects BOUT++ data from parallelized simulations into xarray.
https://xbout.readthedocs.io/en/latest/
Apache License 2.0
22 stars 10 forks source link

Fix whitespace in exception message #123

Closed ZedThree closed 4 years ago

ZedThree commented 4 years ago

Related question, is it loading a single dump file from a parallel run not supported?

codecov-io commented 4 years ago

Codecov Report

Merging #123 into master will not change coverage. The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #123   +/-   ##
=======================================
  Coverage   50.29%   50.29%           
=======================================
  Files          11       11           
  Lines        1175     1175           
  Branches      234      234           
=======================================
  Hits          591      591           
  Misses        512      512           
  Partials       72       72           
Impacted Files Coverage Δ
xbout/load.py 75.19% <0.00%> (ø)

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 8d5f7af...e9dc174. Read the comment docs.

johnomotani commented 4 years ago

There's a workaround here https://github.com/boutproject/xBOUT/blob/e9dc174eb3c6cff9873ca4af65c6e285d4c5c103/xbout/load.py#L388-L406 to detect if there's a single output file (e.g. from parallel output, or from having squashed the output into a single file). It also works (I use it routinely) with a set of files containing consecutive time-chunks of a run, but with the spatial domain squashed. It feels a bit hacky to me, so more elegant suggestions welcome, but works (at least for what I've wanted it for).

johnomotani commented 4 years ago

Related to the fixing whitespace, I'd be in favour of running black over the whole project, also fixing anything flake8 complains about, and then adding them to the CI tests. Maybe we could merge #107, #108, #117 and this PR, and then have a one-shot update of the formatting?

ZedThree commented 4 years ago

I did run black on this file, and was a bit surprised to find lots of changes! I thought black was getting run automatically already. Am I getting mixed up with hypnotoad maybe?

ZedThree commented 4 years ago

Oh, sorry, the whitespace fixed here is inside the string, so wouldn't be touched by black. The printed error looked like:

ValueError: Each run directory does not contain an equal numberof output files. If the parallelization scheme of your simulation changed partway-through, then pleaseload each directory separately and concatenate themalong the time dimension with xarray.concat().
johnomotani commented 4 years ago

I did run black on this file, and was a bit surprised to find lots of changes! I thought black was getting run automatically already. Am I getting mixed up with hypnotoad maybe?

Maybe! hypnotoad does have auto-checking with black, xBOUT does not (yet).