feat: prototype to support multiple prefixes #225
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #225 +/- ##
==========================================
+ Coverage 85.54% 88.47% +2.92%
==========================================
Files 18 18
Lines 1128 1171 +43
Branches 220 102 -118
==========================================
+ Hits 965 1036 +71
+ Misses 153 119 -34
- Partials 10 16 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
nayib-jose-gloria
changed the title
| @@ -2,6 +2,10 @@ | |||
| "$schema": "http://json-schema.org/draft-07/schema#", | |||
| "title": "Ontology Version and Source Schema", | |||
| "definitions": { | |||
| "AfPO_term_id": { | |||
There was a problem hiding this comment.
It would be nice to not have to update this every time we add a new prefix. Ideas welcome
There was a problem hiding this comment.
It doesn't hurt to have an index of all the terms we parse for + explicitly enforce we're not including anything we don't want to. I'm fine to keep it as it is
There was a problem hiding this comment.
i'm not entirely sure what the alternative would be, but i think i would slightly prefer having an explicit allowlist of prefixes we parse for
| @@ -266,8 +267,8 @@ def _parse_ontologies( | |||
| version = ontology_info[onto.name]["version"] | |||
| output_file = os.path.join(output_path, get_ontology_file_name(onto.name, version)) | |||
| logging.info(f"Processing {output_file}") | |||
|
|
|||
| onto_dict = _extract_ontology_term_metadata(onto) | |||
| allowed_ontologies = [onto.name] + ontology_info[onto.name].get("additional_ontologies", []) | |||
There was a problem hiding this comment.
Should allowed_ontologies agree with additional_ontologies in the schema?
There was a problem hiding this comment.
I think allowed_ontologies == main ontology + additional ontologies makes intuitive sense as-is
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_ontology(tmp_path): |
There was a problem hiding this comment.
@nayib-jose-gloria thank you for fixing.
🤖 I have created a release *beep* *boop* --- <details><summary>python-api: 1.3.0</summary> ## [1.3.0](python-api-v1.2.0...python-api-v1.3.0) (2024-11-14) ### Features * query cross-ontology terms imported into a supported ontology ([#232](#232)) ([2e1a834](2e1a834)) * update EFO to 3.71.0 ([#242](#242)) ([7a92c0f](7a92c0f)) ### Misc * add test to ensure all supported ontologies are supported by COG API ([#236](#236)) ([56051f9](56051f9)) </details> <details><summary>ontology-assets: 1.2.0</summary> ## [1.2.0](ontology-assets-v1.1.0...ontology-assets-v1.2.0) (2024-11-14) ### Features * prototype to support multiple prefixes ([#225](#225)) ([dbcdd29](dbcdd29)) * update EFO to 3.71.0 ([#242](#242)) ([7a92c0f](7a92c0f)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Reason for Change
Changes
additional_term_prefixto ontology_info schemaadditional_term_prefixin_extract_ontology_term_metadataTesting steps
Notes for Reviewer