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

Check whether the grid is the one from the simulation #246

Merged
merged 7 commits into from
Oct 25, 2022

Conversation

dschwoerer
Copy link
Contributor

No description provided.

@pep8speaks
Copy link

pep8speaks commented Sep 8, 2022

Hello @dschwoerer! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 63:89: E501 line too long (90 > 88 characters)

Comment last updated at 2022-09-09 20:09:05 UTC

@dschwoerer
Copy link
Contributor Author

* In the file [`xbout/load.py`](https://github.com/boutproject/xBOUT/blob/b83c4141571f124d7f381d8664767024d2bdbb75/xbout/load.py):

Line 63:89: E501 line too long (90 > 88 characters)

That was actually changed by black. I can also remove the comment, but thought the it might be useful if we ever want to add static typing infos

@ZedThree
Copy link
Member

ZedThree commented Sep 8, 2022

We should turn off that flake8 warning, and let black handle line length

@johnomotani
Copy link
Collaborator

johnomotani commented Sep 8, 2022

We should turn off that flake8 warning, and let black handle line length

We have, but flake8-5.0.x seems to be ignoring the config in our .flake8, even though it shouldn't be (as far as I could see from changelog and issues)...

Edit: sorry, rather we've set flake8 to use the same line length as black, but you do have a point as there are some situations where black allows a line length >88 (e.g. in comments) that flake8 complains about.

Copy link
Collaborator

@johnomotani johnomotani left a comment

Choose a reason for hiding this comment

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

Nice! I like this 👍

xbout/load.py Outdated Show resolved Hide resolved
xbout/load.py Outdated Show resolved Hide resolved
@dschwoerer
Copy link
Contributor Author

Thanks for the fixes 👍

@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2022

Codecov Report

Merging #246 (40c0f5c) into master (e8983ca) will decrease coverage by 0.17%.
The diff coverage is 30.00%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
- Coverage   73.99%   73.81%   -0.18%     
==========================================
  Files          13       13              
  Lines        2507     2517      +10     
  Branches      596      608      +12     
==========================================
+ Hits         1855     1858       +3     
- Misses        431      435       +4     
- Partials      221      224       +3     
Impacted Files Coverage Δ
xbout/load.py 73.46% <30.00%> (-1.10%) ⬇️
xbout/plotting/animate.py 46.08% <0.00%> (ø)
xbout/plotting/plotfuncs.py 48.27% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

xbout/load.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@johnomotani johnomotani left a comment

Choose a reason for hiding this comment

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

Thanks @dschwoerer!

xbout/load.py Outdated Show resolved Hide resolved
@johnomotani johnomotani merged commit 8cd6781 into master Oct 25, 2022
@johnomotani johnomotani deleted the gridmismatch branch October 25, 2022 11:15
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.

5 participants