Replace plottertypes by Singleton#109
Conversation
310c9cb to
ad92376
Compare
|
Had a quick search on GitHub and all that came up were forks of GridVisualize or completely unrelated projects when it comes to MakieType. |
|
I think formally this would be a breaking change which by semver increases the major version number and we very much should try to stick to this. I would put this on a list with other possibly breaking changes, which together would be substantial enough to be worth a najor release. |
|
Ok, had a closer look. I was first surprised by the still would return |
|
The other surprise is the use of Union types here. Indeed, they give the correct subtype relations. However |
|
For the formal semver consistency we could just set |
|
I think the PR then also should replace the implementations of the |
40ba322 to
d87c6bb
Compare
|
@j-fu I have rewritten the isxyz functions to use |
|
Also, in regards to #110 I have moved all the deprecated things to a dedicated |
7885bd8 to
9220940
Compare
|
Ok I checked this out and test this in a couple of projects. So far, it works, and I think it should generally be ok. However I think that we should not deprecate |
|
@j-fu isxyz have been undeprecated so I think it should be ready to merge |
|
Ok, LGTM. Thank you! The CI problems seem to be due to some python problems on linux and should be addressed in a different PR. |
As proposed in #108 this changes dispatch.jl to use Singletons instead of using the set of nested ifs in
plottertypewith ismakie() and similar.Since
MakieTypewas heuristic based I had to introduce the alternativeAbstractMakieTypeand change the Makie extension accordingly by addingwhere {MakieType <: AbstractMakieType}where needed.Then all tests ran locally without problem.
Now, extending GridVisualize with another plotting backend can be done without needing to touch the main package.
Nevertheless, introducing a new shorthand through
const AnotherPlotterPackageType = PlotterType{:AnotherPlotterPackage}is certainly nice and quickly done.
@j-fu do you know of any downstream package that might explicitly use
MakieType?And if you are fine with this we should test the alternative to the example to remove the !private! functions ismakie and similar.