Add TrafilaturaExtractor class
#431
Conversation
Signed-off-by: Sarah Yurick <[email protected]>
Signed-off-by: Sarah Yurick <[email protected]>
| expected = [ | ||
| "Let's keep this paragraph: either came does last new took taken making became from.", | ||
| ] |
There was a problem hiding this comment.
Trafilatura has a really bad bug where it is returning the string twice. I can double check all of my logic, but in the case that this is a Trafilatura-specific issue, I am debating whether I should add our own exact deduplication code into this.
(Trafilatura actually has their own deduplicate parameter here, but it did not fix this use case.)
Possible related issue: adbar/trafilatura#634
There was a problem hiding this comment.
I have not scoped to see what conditions cause this.
There was a problem hiding this comment.
Another issue: adbar/trafilatura#768
There was a problem hiding this comment.
Solved by adding support for these extraction and deduplication parameters: https://trafilatura.readthedocs.io/en/latest/settings.html.
There was a problem hiding this comment.
This seems like an issue customers might run into. Have you looked in papers and such for good default values? If Trafilatura's defaults aren't good (or aren't used by most researchers), it could be good to substitute our own.
There was a problem hiding this comment.
Yeah, I can look into it. At the very least, I think setting deduplicate=True as the default in NeMo Curator might be a good idea. Otherwise, I think we could make sure to emphasize these parameters in the documentation.
Signed-off-by: Sarah Yurick <[email protected]>
Signed-off-by: Sarah Yurick <[email protected]>
Signed-off-by: Sarah Yurick <[email protected]>
Signed-off-by: Sarah Yurick <[email protected]>
| @@ -128,15 +131,15 @@ You can choose to modify the HTML text extraction algorithm used in ``download_c | |||
| if __name__ == "__main__": | |||
| main() | |||
|
|
|||
| Above, we changed the extraction algorithm from the default ``JusTextExtractor``. | |||
| Above, we changed the extraction algorithm from the default ``JusTextExtractor``. **Note:** Please see the Trafilatura documentation `here <https://trafilatura.readthedocs.io/en/latest/settings.html>`_ | |||
There was a problem hiding this comment.
Why should the user see the Trafilatura documentation?
There was a problem hiding this comment.
I think this was a lingering comment meant to explain what the different Trafilatura-specific parameters mean. I will remove it in favor of the class docstring suggested below.
| @@ -200,6 +203,98 @@ def extract_text(self, html, stop_words): | |||
| return result | |||
|
|
|||
|
|
|||
| class TrafilaturaExtractor(HTMLExtractorAlgorithm): | |||
There was a problem hiding this comment.
Can you add a class level docstring explaining what trafilatura is?
| expected = [ | ||
| "Let's keep this paragraph: either came does last new took taken making became from.", | ||
| ] |
There was a problem hiding this comment.
This seems like an issue customers might run into. Have you looked in papers and such for good default values? If Trafilatura's defaults aren't good (or aren't used by most researchers), it could be good to substitute our own.
Closes #400.