fix: Better logic for setting category_depth metadata for Title elements
#1517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
newelh
suggested changes
Sep 26, 2023
newelh
approved these changes
Sep 26, 2023
|
LGTM! |
cragwolfe
reviewed
Sep 27, 2023
cragwolfe
reviewed
Sep 27, 2023
CHANGELOG.md
Outdated
| @@ -15,9 +15,9 @@ | |||
|
|
|||
| ### Fixes | |||
|
|
|||
| * **Fixes category_depth None value for Title elements** Problem: The mapping of `Headline` and `Subheadline` element types from `chipper` to `Title`, assigns metadata `category_depth` 1 and 2 respectively. During the same mapping, the `Title` elements from `chipper` are left unchanged, with metadata `category_depth` = None. Fix: Whenever `Headline` or `Subheadline` type layout elements are present in a page, then all `Title` elements with `category_depth` = None should be set to have a depth of 0 instead. | |||
There was a problem hiding this comment.
does this make sense semantically?
remind me what the category_depth for an H1 element is again? if it is 0, does this imply all other Titles from Chipper are like html H1's?
Please add "Why does it matter? for the 2nd sentence.
There was a problem hiding this comment.
correct, H1 is category_depth 0. For, chipper: Title elements are like H1, Headline like H2, and Subheadline like H3. I am changing the description of the fix for clarification.
add missing bullet in Fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR promotes the
category_depthmetadata forTitleelements fromNoneto 0, wheneverHeadlineand/orSubheadlinetypes (that are also mapped toTitleelements with depth 1 and 2) are present. An additional test totest_common.pyhas been added to check on the improvement. More test of how this logic fixes the behaviour can be found in a adapted version on the colab here.Problem: The mapping of
HeadlineandSubheadlineelement types fromchippertoTitle, assigns metadatacategory_depth1 and 2 respectively. During the same mapping, theTitleelements fromchipperare left unchanged, with metadatacategory_depth= None.Fix: Whenever
HeadlineorSubheadlinetype layout elements are present in a page, then allTitleelements withcategory_depth= None should be set to have a depth of 0 instead.Importance: Title elements should be equivalent html H1 when nested headings are present; otherwise, category_depth metadata can result ambiguous within elements in a page.