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 |
62eeb74 to
7885bd8
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 |
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.