Feature/ega2 catalog - #106
Merged
Merged
Conversation
Contributor
Reviewer's GuideIntroduces an EGA accession-based CLI ingestion path, maps EGA metadata plus node-supplied configuration into Health-RI DCAT, and connects the result to RDF and FAIR Data Point outputs with tests and an example configuration. Sequence diagram for EGA metadata ingestion and catalog outputsequenceDiagram
participant User
participant CLI
participant EGAAPI
participant Mapping
participant RDF
participant FDP
User->>CLI: ega -a dataset_id
CLI->>EGAAPI: fetch_ega_datasets(dataset_ids, api_url)
EGAAPI-->>CLI: Dataset metadata
User->>CLI: map-ega-hriv2
CLI->>Mapping: map_ega_to_healthri_dcat_dataset(ega_dataset, config)
Mapping-->>CLI: HRIDataset
User->>CLI: rdf or fdp
CLI->>RDF: Serialize mapped dataset
CLI->>FDP: Publish mapped dataset
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pyproject.toml" line_range="37" />
<code_context>
"sparqlwrapper~=2.0",
"fairclient >= 1.0.0",
"pandas >= 2.0.0",
+ "requests >= 2.34.2",
]
</code_context>
<issue_to_address>
**issue (bug_risk):** Package installation fails because the dependency requires `requests >= 2.34.2`, a version that is not available in the Requests release line, so the project dependency resolver cannot satisfy the requirement.
**Triggers:** When installing the project from a clean environment.
**Suggested fix:** Use an available Requests version constraint, such as the intended released version or a compatible lower bound.
```suggestion
"requests >= 2.32.0",
```
</issue_to_address>
### Comment 2
<location path="src/img2catalog/mappings/ega.py" line_range="38" />
<code_context>
+def fetch_ega_datasets(dataset_ids: List[str], api_url: str) -> List[Dict]:
+ datasets = []
+ for dataset_id in dataset_ids:
+ try:
+ datasets.append(fetch_ega_dataset(dataset_id, api_url))
+ except requests.RequestException as e:
</code_context>
<issue_to_address>
**issue (bug_risk):** `get_release_date` returns the unparseable string despite its `Optional[datetime]` contract, so `map_ega_to_healthri_dcat_dataset` passes that string to `HRIDataset` and model validation raises instead of producing a dataset for an API response with an invalid release date.
**Triggers:** When an EGA dataset contains a non-ISO `released_date`.
**Suggested fix:** Return `None` after logging the parse failure, or raise a deliberate mapping error before constructing `HRIDataset`; do not return a string from a datetime-typed helper.
```suggestion
return None
```
</issue_to_address>
### Comment 3
<location path="src/img2catalog/inputs/ega.py" line_range="18-22" />
<code_context>
+def fetch_ega_datasets(dataset_ids: List[str], api_url: str) -> List[Dict]:
+ datasets = []
+ for dataset_id in dataset_ids:
+ try:
+ datasets.append(fetch_ega_dataset(dataset_id, api_url))
+ except requests.RequestException as e:
+ logger.warning("Error fetching EGA dataset %s: %s", dataset_id, e)
+
+ return datasets
\ No newline at end of file
</code_context>
<issue_to_address>
**issue (bug_risk):** Request failures are logged and discarded, so the CLI exits successfully with an incomplete or empty dataset list even though one or more explicitly requested accession IDs were not imported.
**Triggers:** When any requested EGA accession returns an HTTP error, times out, or otherwise raises `requests.RequestException`.
**Suggested fix:** Propagate an aggregate error or make the CLI exit nonzero when a requested accession cannot be fetched; if partial results are intentional, report the failed IDs prominently and distinguish the run from a successful import.
</issue_to_address>
### Comment 4
<location path="examples/ega/example_config.toml" line_range="8" />
<code_context>
+theme = ["http://publications.europa.eu/resource/authority/data-theme/HEAL"]
+keyword = ["list", "of", "key", "words"]
+access_rights = "http://publications.europa.eu/resource/authority/access-right/PUBLIC"
+applicable_legislation = ["http://publications.europa.eu/resource/authority/access-right/NON_PUBLIC"]
+
+[dataset.publisher]
</code_context>
<issue_to_address>
**issue:** The example config declares the access-right vocabulary URI `.../access-right/NON_PUBLIC` as applicable legislation, so every generated EGA dataset contains an invalid legislation value rather than a legal-act URI.
**Triggers:** When the documented example configuration is used for the manual EGA-to-RDF or EGA-to-FDP command.
**Suggested fix:** Replace the value with the applicable legal instrument URI intended for the node's datasets.
```suggestion
applicable_legislation = ["http://data.europa.eu/eli/reg/2025/327/oj"]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+18
to
+22
| try: | ||
| datasets.append(fetch_ega_dataset(dataset_id, api_url)) | ||
| except requests.RequestException as e: | ||
| logger.warning("Error fetching EGA dataset %s: %s", dataset_id, e) | ||
|
|
Contributor
There was a problem hiding this comment.
issue (bug_risk): Request failures are logged and discarded, so the CLI exits successfully with an incomplete or empty dataset list even though one or more explicitly requested accession IDs were not imported.
Triggers: When any requested EGA accession returns an HTTP error, times out, or otherwise raises requests.RequestException.
Suggested fix: Propagate an aggregate error or make the CLI exit nonzero when a requested accession cannot be fetched; if partial results are intentional, report the failed IDs prominently and distinguish the run from a successful import.
|
ishtiaqahmad
approved these changes
Aug 28, 2026
ishtiaqahmad
left a comment
ishtiaqahmad
left a comment
There was a problem hiding this comment.
While pushing data to Fair Data Point (FDP), the EGA client defaults to port 80, which is incorrect. This should be fixed, but let's treat it as out of scope for this PR.
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.
Description:
Summary
egaCLI input that fetches dataset metadata from the EGA (European Genome-phenome Archive) metadata API by accession IDmap-ega-hriv2to map EGA dataset metadata into the Health-RI DCAT dataset model, with dataset-level config (theme, publisher, contact point, access rights, applicable legislation) supplied via the img2catalog config filerdfandfdpoutput commands, and adds an example config (examples/ega/example_config.toml)Test plan
tests/img2catalog/inputs/test_ega.py— EGA API fetch input teststests/img2catalog/mapping/test_ega.py— EGA → Health-RI DCAT mapping testsimg2catalog -c examples/ega/example_config.toml ega -a <accession_id> map-ega-hriv2 fdp --fdp <url> --username <user> --password <pass> --catalog <catalog_uri>Summary by Sourcery
Integrate EGA dataset metadata ingestion and Health-RI catalog export into img2catalog.
New Features:
Enhancements:
Build:
Tests: