-
Notifications
You must be signed in to change notification settings - Fork 34
toRdf() and flatten() API require extractAllScripts to default to true #603
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
Labels
Comments
This comment was marked as resolved.
This comment was marked as resolved.
Discussed on [today's call](https://json-ld.org/minutes/2024-07-10/#28)✪#603 -> Issue 603 toRdf() and flatten() API require extractAllScripts to default to true (by gkellogg) [spec:enhancement] [needs discussion] [ErratumRaised] ✪
Anatoly Scherbakov: Dlehn I wouldn't like to distract everyone as this is probably off-topic, but the issue I am facing now with python-yaml-ld is that I am redoing a lot of stuff that pyld already is doing. Basically, pyld already implements everything that's needed except a special DocumentLoader. So ideally, it feels like there shouldn't be a separate library for ✪
Anatoly Scherbakov: YAML-LD at all, it should be just a feature of pyld. But in addition to that I would like to see improvements like pytest-based testing framework and better mypy/typing support. So I am still working on this as a separate implementation but it, kind of, feels wrong. ✪
Gregg Kellogg: Probably need to update the API to default to true, but unfortunate. ✪
Pierre-Antoine Champin: Do we have text which is inconsistent with tests? ✪
Pierre-Antoine Champin: Sounds good to me ✪
Gregg Kellogg: My thought is to change API for toRDF but not flatten. ✪
Ted Thibodeau Jr.: I think tests are editable, if incorrect. ✪
... We should fix the test suite, if its wrong.
... Implementations may need to be changed, but this should have been reported as a bug.
Pierre-Antoine Champin: That's what we have in rdf-canon: https://w3c.github.io/rdf-canon/spec/#h-note-3 ✪
PROPOSAL: update tests for flatten to assume extractAllScripts defaults to false. Update toRdf() API to set default for extractAllScripts to true.
Gregg Kellogg: +1 ✪
Anatoly Scherbakov: +1 ✪
Ted Thibodeau Jr.: +1 ✪
Pierre-Antoine Champin: +1 ✪
Niklas Lindström: +0 (Haven't thought it through fully) ✪
David I. Lehn: +0 ✪
Niklas Lindström: Probably makes sense; so +ITY ;) ✪
|
Summary: Explicitly set the default value for |
gkellogg
added a commit
that referenced
this issue
Jul 10, 2024
gkellogg
added a commit
that referenced
this issue
Jul 10, 2024
Add html/f005 with extractAllScripts set to true. For #603.
gkellogg
added a commit
that referenced
this issue
Oct 25, 2024
Add html/f005 with extractAllScripts set to true. For #603.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The JSON-LD section on JSON-LD in HTML Documents specifically says that all script all script elements are extracted, implying that the default value of |extractAllScripts| should be
true
for thetoRdf()
API entry point.Tests for
flatten()
also say that it should extract all script elements, although there is no normative text to this effect.Changing the behavior of
flatten()
would represent a change to the 1.1 behavior, but may be warranted.The
toRdf()
entry point should be updated to say that the default istrue
.The text was updated successfully, but these errors were encountered: