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

Adjust compat settings and progress named argument #84

Merged
merged 6 commits into from
Jan 10, 2024
Merged

Conversation

dmbates
Copy link
Collaborator

@dmbates dmbates commented Dec 11, 2023

  • Replace resolution named argument with size in Makie calls. Restrict version of Makie to later versions with the size named argument.
  • Bump allowed versions of BSplineKit
  • Use progress named argument in call to parametricbootstrap.

We may want to wait for a new release of CairoMakie before incorporating these changes. There are warnings in the tests that trace back to CairoMakie.

@dmbates dmbates requested a review from palday December 11, 2023 16:21
Project.toml Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

can you also bump the version so that we can immediately tag a release?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe the version has already been bumped to 0.3.28 (last release was 0.3.27). Am I confused?

@palday
Copy link
Owner

palday commented Dec 14, 2023

coefplot! now seems to use the first theme color (blue) instead of black, which is fine on its own but looks weird overlain on the ridgeplot!. Can you add an explicit color=:black to the coefplot! call within ridgeplot!?

Copy link
Collaborator Author

@dmbates dmbates left a comment

Choose a reason for hiding this comment

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

I have added color=:black in the call to coefplot! as you requested.

src/ridge.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@palday
Copy link
Owner

palday commented Dec 25, 2023

@dmbates It seems like there is a bigger problem with recipes in Makie 0.20:

MakieOrg/Makie.jl#3514

I'm going to delay merging this until we get some triage over there. If it looks like recipes are just generally deprecated, I'll rework the implementation to not rely on them.

@dmbates
Copy link
Collaborator Author

dmbates commented Dec 25, 2023

@palday Thanks for checking on that.

Copy link
Collaborator Author

@dmbates dmbates left a comment

Choose a reason for hiding this comment

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

@palday Is there something I can do to help get this PR in shape to commit?

We seem to be in a convoluted dependency situation w.r.t EmbraceUncertainty. We can't use julia v1.10.0 to render that project unless we allow AlgebraOfGraphics v0.6.18, which includes a fix for the dictionaries issue when using density(). But that will require later versions of Makie than MixedModelsMakie currently allows.

I am willing to devote time to this, including rewriting the density plots in EmbraceUncertainty to avoid AlgebraOfGraphics (I am starting to agree with you that it is easier in the long run to call Makie functions directory and avoid AoG), if you can give me guidance on how to proceed.

Copy link
Owner

@palday palday left a comment

Choose a reason for hiding this comment

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

The coef- and ridgeplots are broken because of the issue I linked above, but I think that's an upstream issue. I can start working on a replacement that doesn't rely on recipes, but there's no reason to block this work on that.

@palday palday merged commit 772c89c into main Jan 10, 2024
4 checks passed
@palday palday deleted the db/compat branch January 10, 2024 23:13
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.

2 participants