Repository navigation
fix(jats): keep the license and funding statements of the article - #4866
Conversation
|
✅ DCO Check Passed Thanks @Thanatos9404, all your commits are properly signed off. 🎉 |
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 Require two reviewer for test updatesWhen test data is updated, we require two reviewers
|
AI triage (advisory)Checked commit The JATS backend now reads copyright, license and funding statements from the article metadata. It adds them to the document in the furniture content layer, with their links. CI safety: no concern found. A maintainer must still approve CI. 2 related pull request(s) in the same code
Reference data changes
Assessment: expected. Four JATS reference files gain 4 to 10 text items each. This matches the new furniture items for license, copyright and funding statements. The PR text does not describe other changes. The |
There was a problem hiding this comment.
AI first review (advisory): no blocker found
This review covers the full diff at 8c60271a48. It read the code and did not run it.
I found no defect in the change. The new statements are walked with _walk_linear and emitted with ContentLayer.FURNITURE. They do not enter the default Markdown or text export, and the new tests check this. The reviewer should confirm that furniture is the layer the maintainers want for license and funding text, because the PR author offers to move it to the body.
A maintainer makes the merge decision. This review never approves the pull request.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
AI first review (advisory): no blocker found
This review covers the changes since 8c60271a48 at 01cacfd6d4. It read the code and did not run it.
The new changes are a merge of the base branch. They do not touch the JATS backend, its tests, or its reference data, so the earlier review of the JATS code still holds.
A maintainer makes the merge decision. This review never approves the pull request.
ceberam
left a comment
There was a problem hiding this comment.
Thanks @Thanatos9404 for this improvement to JATS documents.
I left a non-blocking design question.
|
@Thanatos9404 We've done quite a few significant PRs lately. Could you please resolve the conflicts again? |
The backend read only the title, the authors and the abstract from <article-meta>. The copyright and license statements in <permissions> and the funding statements in <funding-group> were dropped, together with their links. Walk each <copyright-statement>, <license-p> and <funding-statement> with _walk_linear, so styling and ext-link targets are kept, and add it to the furniture layer. Other backends put content that is outside the reading order there, such as page headers and footers, so the default Markdown and text exports do not change. The statements are added after the back matter, so the existing items keep their positions. Regenerate the JSON ground truth. The Markdown and indented text ground truth does not change. Resolves docling-project#4794 Signed-off-by: Yashvardhan Thanvi <[email protected]> Signed-off-by: Cesar Berrospi Ramis <[email protected]> Co-Authored-By: Cesar Berrospi Ramis <[email protected]>
Main now keeps the boundary whitespace of JATS inline runs (the InlineGroup whitespace contract), so the text run before the ext-link in the licence statement ends with a space. Signed-off-by: Yashvardhan Thanvi <[email protected]>
…_jats Merge the two tests from test_backend_jats_metadata.py into test_backend_jats.py and delete the old file. Reuse convert_jats_article_meta, which now takes an optional body, instead of a separate helper. Signed-off-by: Cesar Berrospi Ramis <[email protected]>
4a9afd6 to
5d49f20
Compare
ceberam
left a comment
There was a problem hiding this comment.
@Thanatos9404 Since another PR #5216 depends on this one, I moved ahead and rebased and resolved the conflicts.
I have also moved the two tests into the main test file for JATS backend, since a new file was not necessary. This allows us to reuse an existing helper.
Thanks for your contribution 🚀
From
<article-meta>, the JATS backend reads only the title, the authors with their affiliations, and the abstract (_parse_metadata). The copyright and license statements in<permissions>and the funding statements in<funding-group>are never visited, so their text and their links are missing from the document.Issue resolved by this Pull Request:
Resolves #4794
Where the statements go
The issue asks where these statements belong. This PR puts them in the furniture content layer:
<title>as furniture "since it is part of the document metadata", and the content of<footer>, where sites put copyright and license notices, also goes to furniture. The Word and Pages backends put headers, footers and footnotes there, and the ODF backend its footnotes; the iWork backend gives the reason: "available to callers that ask for it but stay out of the reading order by default".export_to_dict(), indoc.textsand in any export that includesContentLayer.FURNITURE.If you prefer them in the body, for example after the abstract under their own headings, that is a small change and I can update the PR.
Change
_parse_statementscollectspermissions/copyright-statement,permissions/license/license-pandfunding-group/funding-statementfrom the article metadata in source order. Like abstract paragraphs, they are kept as nodes and walked at emit time._add_statementswalks each statement with_walk_linear, so<ext-link>targets and styling are kept as in body paragraphs, and emits it under the title with_emit_inlinein the furniture layer. A statement with several runs gets an inline group, also in the furniture layer._emit_inlinetakes an optionalcontent_layer. The existing callers do not pass it and do not change.Scope: I included
<copyright-statement>because it is the other statement in<permissions>and a license is attributed with it. The structured funding data (<award-group>,<funding-source>,<award-id>), thexlink:hrefof<license>and<ali:license_ref>are not converted. They are identifiers rather than statements, andDoclingDocumenthas no document-level metadata field for them. In all four fixtures with a license, the license URL is also the target of an<ext-link>in the license text, so the URL is kept. One known limit: block children of<license-p>(a list, formula, figure or table) are still emitted as body content; none of the fixtures has one.Behavior on the input from the issue:
main):CHARLIE-LICENCE,CHARLIE-FUNDINGand the license URL are not inexport_to_dict().doc.textsgetsCHARLIE-LICENCE terms of the,licence(hyperlinkhttps://example.org/c3-licence),.andCHARLIE-FUNDING statement., all in the furniture layer.export_to_markdown()is unchanged. Withincluded_content_layers={ContentLayer.BODY, ContentLayer.FURNITURE}, the two statements follow the body.Reference data
Only the
.jsonfiles change, and only by additions (503 lines added, none removed). The.mdand.itxtfiles do not change, which confirms that the body is the same.elife-56337: the license (with its CC0 link) and the funding statement.pntd.0008301: the license (CC0 link) and the funding statement (link to usaid.gov).pone.0234687: the license (CC0 link) and the funding statement (links to the three funders).ptag100: the copyright statement and the two license paragraphs (CC BY link, andSCOAP<sup>3</sup>as a superscript run). Its<funding-group>has only an<award-group>, so no funding statement.pmc2231364: no change (no<permissions>or<funding-group>).This PR and #4813 regenerate the same JSON reference files (
pntd.0008301,pone.0234687,ptag100). Whichever merges second needs a rebase and regenerated reference data. I will do that.Checklist:
Review evidence:
New tests in
tests/test_backend_jats_metadata.py.tests/test_backend_jats.pyhas 1494 lines and themax-lineshook allows 1500, so the tests are in a new module that reusesget_converter, astests/test_backend_pptx_charts.pydoes.test_jats_license_and_funding_statements_are_furniture: the input from the issue. Checks the furniture runs and the license link, that the default Markdown has the abstract but not the statements, and that the statements follow the body when furniture is included.test_jats_copyright_and_each_license_paragraph_are_kept_in_order: a copyright statement, two license paragraphs (one with<sup>) and two funding statements, each kept as its own item in source order;<copyright-year>and<award-group>are not converted.Both fail on
main(f690a52) and pass with this change.Commands and results (Windows 11, Python 3.10.20, docling-core 2.100.0, environment from
uv sync --frozen --group dev --all-packages --no-group docs --no-group examples --extra format-xml-jats):uv run pytest tests/test_backend_jats_metadata.pywith the backend ofmainuv run pytest tests/test_backend_jats_metadata.py tests/test_backend_jats.py tests/test_input_doc.py tests/test_backend_optional_dependencies.py tests/test_data_gen_flag.pyDOCLING_GEN_TEST_DATA=1 uv run pytest tests/test_backend_jats.py -k e2euv run prek run --files <changed files>(whatmake validateruns)uv run ty check docling/backend/xml/jats_backend.pymainNot run: the full test suite and the ML suites. On Windows, the hooks that call
python3ran through apython3alias of the project interpreter.