Merged
Conversation
📝 WalkthroughWalkthroughAdded license support to the ZBMath importer: configuration tag added, license extraction in the source, a license→Wikibase-QID mapping, publication constructor now accepts licenses, publication creation adds license (P163) claims, and arXiv items are linked back (P1676) when arXiv IDs are present. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as ZBMathSource
participant Pub as ZBMathPublication
participant WB as Wikibase
Source->>Pub: create(..., arxiv_id?, licenses?, ...)
Pub->>WB: Create publication item
WB-->>Pub: Return publication Q-ID
alt arxiv_id present
Pub->>WB: Search for arXiv entities
WB-->>Pub: Return matching arXiv items
Pub->>WB: Add P1676 claim to arXiv items (preprint of → publication Q-ID)
WB-->>Pub: Confirm write
end
alt licenses provided
Pub->>WB: Resolve license URLs via license_dict → Q-IDs
Pub->>WB: Add P163 claims (license) to publication item
WB-->>Pub: Confirm claims added
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mardi_importer/mardi_importer/zbmath/ZBMathPublication.py`:
- Around line 198-203: The loop over self.licenses uses license_dict[l] directly
and will KeyError on unknown URLs; change it to guard lookups by checking
membership (or using .get) before calling self.api.get_claim so unmapped license
URLs are skipped (and optionally logged) rather than aborting the import; keep
using self.api.get_claim(...) and accumulate claims into license_claims before
calling self.item.add_claims(license_claims).
- Around line 148-154: The P1676 "is preprint of" claim is never added for arXiv
records because publication.create() isn't called for them in
ZBMathSource.push(); update the arXiv branch so it adds the same P1676 link when
creating or updating arXiv items. Specifically, in ZBMathSource.push() where you
call create_arxiv_item() or update an existing arXiv item, after obtaining the
arXiv item object add the P1676 claim pointing to publication_id and write the
item (the same logic currently in ZBMathPublication.create()); alternatively,
modify create_arxiv_item() to accept the publication_id and create the P1676
claim there; ensure you use publication.is_arxiv(), create_arxiv_item(), and the
arxiv_item.add_claim/write() calls so the preprint relation is always created or
updated.
In `@mardi_importer/mardi_importer/zbmath/ZBMathSource.py`:
- Line 351: In process_data(), harden the handling of row["license"] before
calling literal_eval and join: check for None or empty string and treat as empty
list; if the value is a plain text fallback (or does not look like a serialized
iterable) treat it as a single-element list; otherwise attempt
literal_eval(row["license"]) inside a try/except (catch SyntaxError/ValueError)
and fall back to treating the original string as a single item; finally set
record["license"] = ";".join(parsed_list) (or "" for empty list). Reference
symbols: row["license"], literal_eval, record["license"], process_data().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6624f5de-9f66-41ad-8854-8cf8ae0923dd
📒 Files selected for processing (3)
config/import_config.config.templatemardi_importer/mardi_importer/zbmath/ZBMathPublication.pymardi_importer/mardi_importer/zbmath/ZBMathSource.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mardi_importer/mardi_importer/zbmath/ZBMathPublication.py (1)
198-207: Rename ambiguous variableltolicense_url.The KeyError hardening looks good. However, static analysis flags
las an ambiguous variable name (E741) — it resembles1in many fonts.♻️ Suggested fix
if self.licenses: license_claims = [] - for l in self.licenses: - license_qid = license_dict.get(l) + for license_url in self.licenses: + license_qid = license_dict.get(license_url) if not license_qid: continue claim = self.api.get_claim("P163", license_qid) license_claims.append(claim) if license_claims: self.item.add_claims(license_claims)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mardi_importer/mardi_importer/zbmath/ZBMathPublication.py` around lines 198 - 207, Rename the ambiguous loop variable `l` to a descriptive name like `license_url` in the ZBMathPublication license-processing block: update the for-loop header (for l in self.licenses) and all uses of `l` to `license_url`, preserving the existing logic that looks up license_qid via license_dict.get(license_url), skips missing keys, calls self.api.get_claim("P163", license_qid), collects claim into license_claims, and finally calls self.item.add_claims(license_claims) if any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mardi_importer/mardi_importer/zbmath/ZBMathPublication.py`:
- Around line 198-207: Rename the ambiguous loop variable `l` to a descriptive
name like `license_url` in the ZBMathPublication license-processing block:
update the for-loop header (for l in self.licenses) and all uses of `l` to
`license_url`, preserving the existing logic that looks up license_qid via
license_dict.get(license_url), skips missing keys, calls
self.api.get_claim("P163", license_qid), collects claim into license_claims, and
finally calls self.item.add_claims(license_claims) if any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83d3e0ee-e71e-4266-b2da-318742bd66bb
📒 Files selected for processing (1)
mardi_importer/mardi_importer/zbmath/ZBMathPublication.py
eloiferrer
approved these changes
Apr 10, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
add license and set arxiv item as preprint of
Summary by CodeRabbit