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.

Sign up for GitHub

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

Jump to bottom

fix vartheta bug #480

Merged
merged 5 commits into from
Apr 6, 2023
Merged

fix vartheta bug #480

merged 5 commits into from
Apr 6, 2023

Conversation

unalmis
Copy link
Collaborator

@unalmis unalmis commented Apr 5, 2023 •
edited
Loading

Resolves #477

dpanici
dpanici requested changes Apr 5, 2023
View reviewed changes
Copy link
Collaborator

@dpanici dpanici left a comment

Choose a reason for hiding this comment

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

Can you add a test_plot_surfaces_HELIOTRON so we can catch changes like this in the future?

ddudt
ddudt previously approved these changes Apr 5, 2023
View reviewed changes
@codecov
Copy link

codecov bot commented Apr 5, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: +0.01 🎉

Comparison is base (4a32a49) 93.84% compared to head (c9807d1) 93.86%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   93.84%   93.86%   +0.01%     
==========================================
  Files          72       72              
  Lines       15022    15020       -2     
==========================================
  Hits        14098    14098              
+ Misses        924      922       -2
Impacted Files Coverage Δ
desc/grid.py 98.19% <ø> (-0.01%) ⬇️
desc/plotting.py 87.91% <ø> (+0.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@unalmis unalmis marked this pull request as ready for review April 5, 2023 07:09
unalmis
unalmis commented Apr 5, 2023
View reviewed changes
desc/grid.py Show resolved Hide resolved
dpanici
dpanici requested changes Apr 5, 2023
View reviewed changes
Copy link
Collaborator

@dpanici dpanici left a comment

Choose a reason for hiding this comment

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

I'd like to see a warning for the surface integral case that could give wrong answers, but besides that looks great

@unalmis unalmis requested a review from dpanici April 5, 2023 17:46
@unalmis unalmis requested a review from ddudt April 5, 2023 23:54
@unalmis unalmis merged commit 07170de into master Apr 6, 2023
@unalmis unalmis deleted the vartheta_plot branch April 6, 2023 18:54
@unalmis unalmis added the bug fix Something was fixed label Jul 22, 2024
@unalmis unalmis self-assigned this Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@f0uriest f0uriest f0uriest approved these changes

@dpanici dpanici dpanici approved these changes

@ddudt ddudt ddudt approved these changes

Assignees

@unalmis unalmis

Labels
bug fix Something was fixed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Plot vartheta lines look off on latest master
4 participants
@unalmis @f0uriest @ddudt @dpanici