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

Compatibility with Makie 0.17 #78

Merged
merged 4 commits into from
May 12, 2022
Merged

Compatibility with Makie 0.17 #78

merged 4 commits into from
May 12, 2022

Conversation

asinghvi17
Copy link
Member

Fixes a small error which caused some graphs to be unusable. I ran into this when trying to use OSMMakie.

@hexaeder
Copy link
Collaborator

this breaks every test with arrows in plots. weird... what happend there?

@asinghvi17
Copy link
Member Author

Huh...just ran this locally, I have no idea what is going on - it seems like the marker size might have blown up or something.

@SimonDanisch
Copy link
Member

Weird indeed.. I thought it's maybe an old Makie version, but the tests seem to use 0.16.6.
And it seems like the size defaults to the scatter theme size, which I suppose should be in pixels?

@hexaeder
Copy link
Collaborator

I am wondering how Pixel ended up there in the first place. Was this a valid option before the markerspace update MakieOrg/Makie.jl#1596? Also the docs mention Makie.spaces() for valid options but this functions does not seem to exist anymore.

@SimonDanisch
Copy link
Member

Huh, maybe actually the changes that introduced spaces and deprecated Pixel may not even be tagged yet, when I think about it.

@hexaeder
Copy link
Collaborator

Makie@0.17 seems to change the glyph placement, I've updated the reference images. Documentation won't run because it can't find some lib used in Jump... not sure what to do there

@asinghvi17 asinghvi17 closed this May 11, 2022
@asinghvi17 asinghvi17 reopened this May 11, 2022
@asinghvi17 asinghvi17 changed the title Pixel -> :pixel in markerspace Compatibility with Makie 0.17 May 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2022

Codecov Report

Merging #78 (b8ba9ad) into master (2434a0b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #78   +/-   ##
=======================================
  Coverage   78.88%   78.88%           
=======================================
  Files           4        4           
  Lines         521      521           
=======================================
  Hits          411      411           
  Misses        110      110           
Impacted Files Coverage Δ
src/recipes.jl 98.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 2434a0b...b8ba9ad. Read the comment docs.

@asinghvi17
Copy link
Member Author

Looks like some JLLs didn't compile - Cbc_jll and Cgl_jll. This caused upstream libraries to fail.

@hexaeder hexaeder merged commit 8bae63f into master May 12, 2022
@hexaeder hexaeder deleted the as/pixeltype branch May 12, 2022 12:41
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.

4 participants