Skip to content

Conversation

@fhagemann
Copy link
Collaborator

We might wanna disallow [email protected] because it doesn't seem to capture units and if a config file is defined with units, they will get lost in the translation, and this messes up config files when writing to a file.

using JSON, Unitful
@show JSON.json(1u"m")
@show JSON.parse(JSON.json(1u"m"))

Using [email protected]:

JSON.json(1 * u"m") = "{\"val\":1}"
JSON.parse(JSON.json(1 * u"m")) = Dict{String, Any}("val" => 1)

Using [email protected]:

JSON.json(1 * u"m") = "\"1.0 m\""
JSON.parse(JSON.json(1 * u"m")) = "1.0 m"

I cannot guarantee that every config dictionary written with LegendHDF5IO using [email protected] will give the correct Simulation when being read in again.

@fhagemann fhagemann added dependencies Pull requests that update a dependency file bug Something isn't working labels Nov 26, 2025
@fhagemann fhagemann requested a review from oschulz November 26, 2025 19:21
@fhagemann fhagemann added this to the v0.11.0 milestone Nov 26, 2025
@fhagemann fhagemann merged commit f3e6a42 into main Nov 28, 2025
10 checks passed
@fhagemann fhagemann deleted the json branch November 28, 2025 16:39
@oschulz
Copy link
Member

oschulz commented Dec 9, 2025

This broke compatibility with PlotlyJS.jl, which is still limited to JSON 0.21.4.

@oschulz
Copy link
Member

oschulz commented Dec 9, 2025

It also broke Juleana, which (indirectly) requires BlackBoxOptim, which also has no JSON v1 compatible release yet.

@fhagemann
Copy link
Collaborator Author

fhagemann commented Dec 9, 2025

Well, for SolidStateDetectors, we're getting wrong results if the config file/dict has units and are saved to a HDF5 file.. If we allow for [email protected], there's a good chance that the saved output is wrong.
Can we push the packages to allow for JSON@1 and migrate away from [email protected]? I would strongly advocate for that, especially when dealing with values with units..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants