interoperability for Stipple.JSONText and JSON.JSONText

[hhaensel]

Only after fixing handling of Stipple.JSONText in StipplePlotly I understood the root cause of the error.

It is that Stipple's definition of JSONText for JSON3 doesn't interoperate with JSON's definition, but PlotlyBase's JSON3-rendering is calling JSON behind the scenes. Therefore, Stipple.JSONText("test") is rendered as "{\"s\": \"test\"}

julia> JSON.json(Stipple.JSONText("test"))
"{\"s\":\"test\"}"

This PR adds all necessary methods to JSON3 and JSON to grant interoperability of the JSONText definitions so that

julia> JSON.json(Stipple.JSONText("test"))
"test"

Respective tests are added to runtests.jl
So if in future some component adds JSON3 rendering via a JSON fallback, we are save to use our Stipple.JSONText.

[essenciary]

@hhaensel is DataFrames a dependency of Stipple now? Is it just for testing? If it's for testing, last time I checked the best practice was to set up a distinct project in the test/ folder and add test dependencies there.

[essenciary]

Same for Test - if it's not used by Stipple itself, it should go into the test/ project. (Unless the best practice has changed recently).

[hhaensel]

AFAIK there are two ways of including packages for testing. One is defining a separate Project.toml in /test, as you said, the other - old school - variant is adding them to the [extras] section and listing their names under targets.

[extras]
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
OffsetArrays = "6fe1bfb0-de20-5000-8ca7-80f57d26f881"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Test", "DataFrames", "JSON", "OffsetArrays"]

I took this practice from the Extensions docs for the case that we want to support old julia versions which do not support extensions.
Re-reading the docs I found probably a way to get rid of the file copies without using, following this hint from the link above.

isdefined(Base, :get_extension) ? (using Contour) : (using ..Contour)

Let's see whether I can get that running.

[hhaensel]

@hhaensel is DataFrames a dependency of Stipple now? Is it just for testing? If it's for testing, last time I checked the best practice was to set up a distinct project in the test/ folder and add test dependencies there.

Here are the docs for test-specific dependencies.

[essenciary]

@hhaensel is DataFrames a dependency of Stipple now? Is it just for testing? If it's for testing, last time I checked the best practice was to set up a distinct project in the test/ folder and add test dependencies there.

Here are the docs for test-specific dependencies.

Thanks for the link. I thought the "old" way was deprecated and was concerned that the dependencies would leak into the package installation. Looks like it's still recommended - though I haven't tested exactly how and when are test deps installed with the current approach, hopefully only when running tests.

[hhaensel]

I think I remember, why the target-approach is used for Requires-compatibility. If you want to set compat entries for your weak dependencies old versions of Julia will complain if the packages are not listed either under [deps] or [extras].

[essenciary]

I think I remember, why the target-approach is used for Requires-compatibility. If you want to set compat entries for your weak dependencies old versions of Julia will complain if the packages are not listed either under [deps] or [extras].

Hmmm... ok. I can't remember though, why do we have table rendering in Stipple? Shouldn't it be in StippleUI?

[hhaensel]

The answer is, we don't. The file StippleTablesExt.jl is mainly empty and never used. It's probably a left over from early testing.

[hhaensel]

@essenciary should we merge this? We could handle improved extension management in a separate PR.

EDIT: I decided that it is better to separate things into two PRs. As soon as this one is merged we can make another PR to merge hh-extension2

[hhaensel]

Meanwhile, I released StipplePlotly 0.13.9 where rendering of Stipple.JSONText is at least fixed for the particular case of PlotlyBase
For a general solution, I would need the ok this PR.

[essenciary]

@hhaensel We'll run some tests to see if anything breaks. FYI @PGimenez

[PGimenez]

I tried a few apps and they all work as expected. I think we can merge.

[hhaensel]

Perfect! Don't trigger new release yet, there's another PR in the pipeline.