feat: add streaming of rdf xml dataset with resume functionality #44
Merged
aditya
merged 4 commits from 2026-01-14 14:24:07 +00:00
streaming-xml into stream-download-convert-upload-merge-15-refactor-rdf-converters
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
Blocked
Needs feedback
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
A ticket in a blocked state and unable to complete until some other task is completed first.
Bounty
$100
A bounty of $100 for any open-source contributor who provides a MR that solves this issue
Bounty
$1000
A bounty of $1000 for any open-source contributor who provides a MR that solves this issue
Bounty
$10000
A bounty of $10000 for any open-source contributor who provides a MR that solves this issue
Bounty
$20
A bounty of $20 for any open-source contributor who provides a MR that solves this issue
Bounty
$2000
A bounty of $2000 for any open-source contributor who provides a MR that solves this issue
Bounty
$250
A bounty of $250 for any open-source contributor who provides a MR that solves this issue
Bounty
$50
A bounty of $50 for any open-source contributor who provides a MR that solves this issue
Bounty
$500
A bounty of $500 for any open-source contributor who provides a MR that solves this issue
Bounty
$5000
A bounty of $5000 for any open-source contributor who provides a MR that solves this issue
Bounty
$750
A bounty of $750 for any open-source contributor who provides a MR that solves this issue
MoSCoW
Could have
Could have feature in order to satisfy the epic/legendary.
MoSCoW
Must have
Must have feature in order to satisfy the epic/legendary.
MoSCoW
Should have
Should have feature in order to satisfy the epic/legendary.
There are questions in the ticket that can not be completed until the project owner provides clarity.
Points
1
1 man-hours worth of work for an expert with no learning curve.
Points
13
13 man-hours worth of work for an expert with no learning curve.
Points
2
2 man-hours worth of work for an expert with no learning curve.
Points
21
21 man-hours worth of work for an expert with no learning curve.
Points
3
3 man-hours worth of work for an expert with no learning curve.
Points
34
34 man-hours worth of work for an expert with no learning curve.
Points
5
5 man-hours worth of work for an expert with no learning curve.
Points
55
55 man-hours worth of work for an expert with no learning curve.
Points
8
8 man-hours worth of work for an expert with no learning curve.
Points
88
88 man-hours worth of work for an expert with no learning curve.
Priority
Backlog
This ticket has backlogged priority and is not to be worked on yet
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
A ticket for learning a tool or technology that is needed to be able to do future planning and design.
State
Completed
The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.
State
Duplicate
A ticket that represents the same content as an existing ticket.
State
In Progress
A ticket that is actively being developed.
State
In Review
A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.
State
Paused
This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.
State
Unverified
All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.
State
Verified
The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.
State
Wont Do
This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.
Type
Bug
Something that doesnt work as intended.
Type
Discussion
Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.
Type
Documentation
An error or improvement needed in the documentation.
Type
Epic
Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.
Type
Feature
Some new functionality not present.
Type
Legendary
A type of Epic which will contain other Epics.
Type
Support
Someone needs help using the project.
Type
Task
A generic task that doesnt fit into the other type categories.
Type
Testing
Work exclusively focusing on fixing or expanding testing.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cleverdatasets/dataset-uploader#44
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "streaming-xml"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Add functionality to stream rdf/xml dataset, it reads line-by-line looking for complete rdf:Description blocks, parse each block to extract triples.
The most important problem: Please use a real XML parser instead of trying to parse files directly.
I recommend looking at pull parsers in Python. Two that I found immediately were
lxml.etree.iterparseandxml.etree.ElementTree.iterparse(standard library), but feel free to use any XML parser that you find easy.@@ -220,0 +222,4 @@xml_declaration: str | None,rdf_root: str | None,description_lines: list[str],) -> str:If you make the below changes, there always will be
xml_declarationandrdf_rootlines. So you won't need the| Noneof thexml_declarationandrdf_rootdeclaration, and you won't need lines 236-245.Removed the direct parse logic, and implemented the parsing using xml.etree.ElementTree
@@ -220,0 +311,4 @@depth = 0for line in lines:line_count += 1It's more Pythonic to combine lines 313-314 as:
especially since you don't use
line_countoutside the loop.Fixed !!
@@ -220,0 +321,4 @@# Extract XML declaration (line 1)if stripped.startswith("<?xml"):root_line = linecontinueI'm VERY, VERY nervous when we try to do our own parsing of something as complex as https://www.w3.org/TR/2006/REC-xml11-20060816/ .
For example, lines 322 parses correctly-formed XML files... but it also allows XML files that incorrectly have multiple prologs.
We should use a real XML parser.
Fixed !!
@@ -220,0 +326,4 @@# Extract root RDF element with namespaces (line 2)if stripped.startswith("<rdf:RDF") and rdf_root_line is None:rdf_root_line = linecontinueI'm very nervous about line 327-329.
This will definitely ONLY work with uniprot files.
I recommend using a real XML parser and asking it to read the namespace. Especially since this code wouldn't work if the
rdf:RDFnamespace were split over several lines.Fixed !!
@@ -220,0 +334,4 @@depth = 1current_description = [line]# Handle self-closing tags: <rdf:Description ... />if stripped.rstrip().endswith("/>"):Why the
rstrip()? Line 315 writes that it was already stripped.Fixed !!
@@ -220,0 +349,4 @@if description_count >= chunk_size:yield triple_batchtriple_batch = []description_count = 0Lines 341-352 and lines 385-401 are very similar. Instead of duplicating, could they be turned into a method?
Fixed !!
@@ -220,0 +358,4 @@# Count opening/closing tags to track depth correctly# Opening tags: <tag> or <tag ...> (not </tag or <tag/>)# Find all <tag patterns (this excludes </tag> because regex requires [a-zA-Z_] after <)ruff checkreports:Fixed !!
@@ -220,0 +359,4 @@# Count opening/closing tags to track depth correctly# Opening tags: <tag> or <tag ...> (not </tag or <tag/>)# Find all <tag patterns (this excludes </tag> because regex requires [a-zA-Z_] after <)all_tag_starts = re.findall(r'<([a-zA-Z_][\w:.-]*)', stripped)Lines 362, 364, and 367 count tags that happen inside comments.
XML is complex. Really. Please fix this by using a real XML parser instead of trying to parse XML directly.
@@ -220,0 +364,4 @@closing_tags = len(re.findall(r'</([a-zA-Z_][\w:.-]*)>', stripped))# Self-closing tags: <tag ... /> (these don't change depth)# Find tags that end with /> before the next >self_closing_tags = len(re.findall(r'<([a-zA-Z_][\w:.-]*)[^>]*/>', stripped))ruff checkreports:Fixed !!
@@ -220,0 +367,4 @@self_closing_tags = len(re.findall(r'<([a-zA-Z_][\w:.-]*)[^>]*/>', stripped))# Opening tags = all tag starts - self-closing tags# (all_tag_starts already excludes closing tags, so we only subtract self-closing)ruff checkreports:Fixed !!
@@ -220,0 +377,4 @@# Check if Description element is closedif "</rdf:Description>" in stripped:# The closing tag was already accounted for in closing_tags aboveif depth == 0:ruff checkreports:Fixed !!
@@ -220,0 +405,4 @@if line_count % PROGRESS_LOG_INTERVAL == 0:console.print(f"[dim]Processed {line_count:,} lines, {description_count:,} descriptions...[/dim]"ruff checkreports:Fixed !!
@@ -227,3 +435,3 @@Args:lines: Iterator yielding text lines of RDF dataformat: RDF serialization format (supported: nt, ntriples, turtle, ttl)format: RDF serialization format (supported: nt, ntriples, turtle, ttl, xml, rdf, rdfxml)ruff checkreports:Fixed !!
@@ -252,3 +462,3 @@def _pick_zip_member(zf: zipfile.ZipFile, *, prefer_exts: tuple[str, ...] = (".ttl", ".nt")) -> str:def _pick_zip_member(zf: zipfile.ZipFile, *, prefer_exts: tuple[str, ...] = (".ttl", ".nt", ".rdf", ".xml")) -> str:ruff checkreports:Fixed !!
@@ -989,0 +1206,4 @@"ntriples": "ntriples",}rdf_format = format_mapping.get(registry_format, registry_format)console.print(f"[dim]Format auto-detected from registry: {registry_format} → {rdf_format}[/dim]")ruff checkreports:.Fixed !!
@@ -1001,3 +1233,3 @@}rdf_format = format_map.get(ext, "turtle")console.print(f"[dim]Format inferred from extension: {rdf_format}[/dim]")console.print(f"[dim]Format inferred from extension: {ext} → {rdf_format}[/dim]")ruff checkreports:Fixed !!
@@ -27,0 +41,4 @@if protocol == "ftp":yield from stream_ftp_bytes(url, config=config)elif protocol in ("http", "https"):yield from stream_http_bytes_impl(url, headers=headers, start_byte=start_byte, config=config)ruff checkreports:Fixed !!
@@ -27,0 +43,4 @@elif protocol in ("http", "https"):yield from stream_http_bytes_impl(url, headers=headers, start_byte=start_byte, config=config)else:raise ValueError(f"Unsupported protocol: {protocol}. Supported: http, https, ftp")ruff checkreports:Fixed !!
@@ -76,0 +148,4 @@raise ValueError(f"File not found on FTP server: {url}") from eelif "timed out" in reason.lower():# Retry on timeoutif attempt > cfg.max_retries:I think that this line should be
Do you agree?
@@ -76,0 +150,4 @@# Retry on timeoutif attempt > cfg.max_retries:raisesleep_s = min(cfg.backoff_base_s * (2 ** (attempt - 1)), cfg.backoff_max_s)Because the definition of
sleep_sis a little complex and it's repeated three times (lines 153, 159, and 166), I would rather it be in its own function, in case we want to change how to calculate the sleep time.In other news,
ruff checkreports:Fixed !!
@@ -76,0 +156,4 @@# Other errors: retry if we have attempts leftif attempt > cfg.max_retries:raisesleep_s = min(cfg.backoff_base_s * (2 ** (attempt - 1)), cfg.backoff_max_s)ruff checkreports:Fixed !!
@@ -76,0 +159,4 @@sleep_s = min(cfg.backoff_base_s * (2 ** (attempt - 1)), cfg.backoff_max_s)time.sleep(sleep_s)except Exception as e:ruff checkreports:Fixed !!
EXCELLENT work, Aditya! Thank you for fixing everything.
@@ -220,0 +302,4 @@return triplesexcept Exception as e:logger.error(f"Error parsing Description XML: {e}")logger.debug(f"XML content (first 500 chars): {xml_content[:500]}")Is it possible to get the XML content where the parse error occurred? The specific crash-provoking section may be more relevant than the first 500 of the document, which is likely to be mainly boilerplate anyway.
@@ -21,1 +28,4 @@return min(base_s * (2 ** (attempt - 1)), max_s)def stream_http_bytes(Method name no longer reflects functionality as this is a manager-method supporting both ftp and http now
7724415794to255ccad4d4Looks good to me. i am approving it.