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

Update reference images (against PR #73) #83

Merged
merged 22 commits into from
Jul 18, 2022

Conversation

greimel
Copy link
Collaborator

@greimel greimel commented Jun 10, 2022

This contains two three new commits (I also merged current master into this branch)

  • The first commit replaces the reference images that have improved due to Edgelabelorient #73.
  • The second commit replaces other reference images that fail tests, but haven't changed significantly.
  • The third commit contains the current version of reference image that might still need some adjustments. I added the commit because github has a very nice diff view for images (even for individual commits). The three images of this commit are shown below.

plots.jl-14.png

TO DO: adjust the offset

image

reftests.jl-04.png

TO DO: leave as is?

image

reftests.jl-05.png

TO DO: adjust axis limits so the label is not cut off?

image

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (edgelabelorient@c13be8d). Click here to learn what that means.
The diff coverage is n/a.

@@                Coverage Diff                 @@
##             edgelabelorient      #83   +/-   ##
==================================================
  Coverage                   ?   79.40%           
==================================================
  Files                      ?        4           
  Lines                      ?      539           
  Branches                   ?        0           
==================================================
  Hits                       ?      428           
  Misses                     ?      111           
  Partials                   ?        0           

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 c13be8d...8da3b0a. Read the comment docs.

@hexaeder
Copy link
Collaborator

Seems fine to me. it doesn't really matter how the plots in reftests.jl look, as long as the plottet graph matches the expectation. If you mark your PR as ready i'd be fine with merging it into @filchristou s PR....

@greimel
Copy link
Collaborator Author

greimel commented Jul 18, 2022

Great! I've merged the latest master into this PR. There had been some redundancies with #85.

@greimel greimel marked this pull request as ready for review July 18, 2022 12:41
@@ -1,5 +1,6 @@
[deps]
CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"
Cbc = "9961bab8-2fa3-5c5a-9d89-47fab24efd76"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why cbc?

Copy link
Collaborator Author

@greimel greimel Jul 18, 2022

Choose a reason for hiding this comment

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

No idea how that happened. Will remove.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That line was added by you :-) b8ba9ad

It shows up here because I merged master into this branch.

Copy link
Collaborator

@hexaeder hexaeder Jul 18, 2022

Choose a reason for hiding this comment

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

#78
ah yes, i remeber now, this was a hotfix to adress Makie@0.17 compat...
sorry :D

@hexaeder hexaeder merged commit 71f59be into MakieOrg:edgelabelorient Jul 18, 2022
@greimel greimel mentioned this pull request Jul 18, 2022
@greimel greimel deleted the refimages branch July 18, 2022 13:33
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.

7 participants