-
Notifications
You must be signed in to change notification settings - Fork 259
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
custom mimetype support - add direct JSON #1173
Comments
What is the problem you're trying to solve here? Could you encode the |
I am trying to test out the custom mimetype by changing
instance IHaskellDisplay VegaLiteLab where
display (VLL vl) = let js = LT.unpack (encodeToLazyText (fromVL vl))
in pure (Display [vegalite js])
to
instance IHaskellDisplay VegaLiteLab where
display (VLL vl) = let js = LT.unpack (encodeToLazyText (fromVL vl))
vegalite' = "application/vnd.vegalite.v4+json"
in pure (Display [custom vegalite' js])
and it does not work because the custom message gets encoded as a String,
whereas the vegalite message gets special cased to handle JSON. It is
actually rather wasteful because I convert from Aeson to lazy text to
string, and them the vegalite encoding in displayDataToJson converts back
ti Aeson.
In this case an alternative is to just bump the vegalite mimetype from
v2+ to v4+, but a want to make sure we can handle the next time the
version gets updated (ie use the custom message until ihaskell can get
updated )
Doug
…On Sat, May 16, 2020 at 10:29 PM Vaibhav Sagar ***@***.***> wrote:
What is the problem you're trying to solve here? Could you encode the
Value as a Text and use the existing data types? If it turns out to be
necessary to add a new mimetype we can do that but I'm wary because each
time it breaks backwards compatibility and complicates the interface.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1173 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABW27SP3G2RXZUXZSS6GUTRR5DYRANCNFSM4NDCHBPA>
.
|
Okay, I'd accept a PR adding this additional mimetype. |
I think this might be tricky to implement at least in part because of this line: How would |
#1176 is my initial attempt at this, but without resolving the |
I now am wondering if it would be better to just add an extra parameter to CustomMimetype to allow you to decide how the Text payload is converted to JSON - e.g. just |
Darn it. I was playing around with
but the need instances give me pause (due to |
I've added a second commit which uses a different design (moves the choice for the encoding to the CustumMimetype type). |
It still doesn't look like this new approach fixes the problem with |
Do you want me to switch #1176 back to the original version? |
I don't think it makes a difference, the original version had the same issue. |
Given the way the Read instance works for other types (e.g. hardcoding width / height) I'm personally don't think it is a serious issue (and technically I think the second version is "better" for Read, but not massively so). |
In coming back to this, we have MimeCustom Text which is - I think - processed by this step displayDataToJson (DisplayData mimeType dataStr) =
pack (show mimeType) .= String dataStr What I want is for it to not encode the data as a displayDataToJson (DisplayData (MimeCustom mimeType) dataStr) =
mimeType .= textToJson dataStr
-- pulled out as it's used in several mime types
textToJson :: Text -> Value
textToJson dataStr = fromMaybe (String "") (decodeStrict (Text.encodeUtf8 dataStr) :: Maybe Value) Last time I tried I don't think I was able to do this as a user of |
So, I've created a notebook showing that I can't seem to use the custom mimetype to replicate the vega-lite mimetype because of this. You can find it at https://gist.github.com/DougBurke/11009ad97320e352c21a0d7558bb0b8b but the gist "auto display" doesn't seem to want to work for me. In writing this I did note that the vega-lite mimetype has been bumped up to v4 so that means I personally am not affected by this for the moment (although the vega/vega-lite schema keeps changing so it could be an issue in the future). So I'm probably/hopefully not going to be spending much time on this in the near future. |
Can we add some way to pass JSON through displayDataToJson for the new custom mimetype? Rarndom thoughts are
a) add a separate mimetype - e.g.
MimeCustomJSON Text
- which can then be used indisolayDataToJson
in a similar way to howMimeJson/MmeVegaite/MimeVega
is handledb) or change the MimeCustom type to treat the value in
displayDataToJson
directly as an Aeson ValueThe bit I'd like to be able to handle is to pass a custom mimetype with a Value, not a String, in the followig.
The text was updated successfully, but these errors were encountered: