Skip to content

Commit 846be51

Browse files
turbomamclaude
andcommitted
fix: address Copilot review comments on generate_deprecated_template
- Add cwd= parameter to extract_ids_from_git_ref and _labels_from_tags so git commands run from the repo root regardless of cwd - Use $(abspath $@) in metpo.Makefile deprecated.tsv target so the output path is absolute and unambiguous after cd ../.. - Widen classify_reason range for Era 3 literature-mining terms from 1000001..1000058 to 1000001..1000327 to match the actual retired range - Remove IAO:0100001 and OBSERVATION_REPLACEMENTS artifacts (no 1:1 replacements exist yet — that's tracked in issue #371) - Regenerate src/templates/deprecated.tsv with corrected reasons Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a7e9d25 commit 846be51

3 files changed

Lines changed: 281 additions & 281 deletions

File tree

metpo/scripts/generate_deprecated_template.py

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
- owl:deprecated true
55
- rdfs:label prefixed with "obsolete "
66
- IAO:0000231 (has obsolescence reason)
7-
- IAO:0100001 (term replaced by) where a replacement exists
87
98
The output TSV is consumed by ROBOT to produce components/deprecated.owl,
109
which is merged into the ontology during prepare_release.
@@ -23,15 +22,8 @@
2322
"src/templates/metpo-properties.tsv",
2423
]
2524

26-
# Observation class IDs → flat data property replacements (from #371)
27-
OBSERVATION_REPLACEMENTS: dict[str, str] = {
28-
# Observation classes have no 1:1 replacement — they're a pattern change.
29-
# Use oboInOwl:consider to point at the flat data property parent.
30-
}
31-
3225
# Obsolescence reasons (IAO individuals)
3326
REASON_PLACEHOLDER_REMOVED = "IAO:0000226"
34-
REASON_TERMS_MERGED = "IAO:0000227"
3527
REASON_OUT_OF_SCOPE = "OMO:0001000"
3628

3729

@@ -56,14 +48,15 @@ def extract_ids_from_entity_extract(path: Path) -> set[str]:
5648
return ids
5749

5850

59-
def extract_ids_from_git_ref(ref: str, template_path: str) -> set[str]:
51+
def extract_ids_from_git_ref(ref: str, template_path: str, *, cwd: Path | None = None) -> set[str]:
6052
"""Extract METPO numeric IDs from a template at a given git ref."""
6153
try:
6254
result = subprocess.run(
6355
["git", "show", f"{ref}:{template_path}"],
6456
capture_output=True,
6557
text=True,
6658
check=True,
59+
cwd=cwd,
6760
)
6861
except subprocess.CalledProcessError:
6962
return set()
@@ -102,7 +95,7 @@ def collect_all_ids(repo_root: Path) -> tuple[set[str], set[str]]:
10295
if not tag:
10396
continue
10497
for tmpl in TEMPLATE_PATHS:
105-
all_ids |= extract_ids_from_git_ref(tag, tmpl)
98+
all_ids |= extract_ids_from_git_ref(tag, tmpl, cwd=repo_root)
10699

107100
return current_ids, all_ids
108101

@@ -131,9 +124,13 @@ def _labels_from_extracts(extracts_dir: Path) -> dict[str, tuple[str, str, str]]
131124
return labels
132125

133126

134-
def _labels_from_tags(repo_root: Path) -> dict[str, tuple[str, str, str]]:
135-
"""Collect labels from tagged releases (higher priority — overwrites)."""
136-
labels: dict[str, tuple[str, str, str]] = {}
127+
def _labels_from_tags(repo_root: Path) -> dict[str, tuple[str, str | None, str]]:
128+
"""Collect labels from tagged releases (higher priority — overwrites).
129+
130+
Returns owl_type=None when the template column is not a real owl: type
131+
(e.g., old templates had parent CURIEs in column 3).
132+
"""
133+
labels: dict[str, tuple[str, str | None, str]] = {}
137134
tags_out = subprocess.run(
138135
["git", "tag", "--sort=creatordate"],
139136
capture_output=True,
@@ -152,6 +149,7 @@ def _labels_from_tags(repo_root: Path) -> dict[str, tuple[str, str, str]]:
152149
capture_output=True,
153150
text=True,
154151
check=True,
152+
cwd=repo_root,
155153
)
156154
except subprocess.CalledProcessError:
157155
continue
@@ -162,8 +160,9 @@ def _labels_from_tags(repo_root: Path) -> dict[str, tuple[str, str, str]]:
162160
numeric_id = parts[0].strip().replace("METPO:", "")
163161
label = parts[1].strip() if len(parts) > 1 else ""
164162
raw_type = parts[2].strip() if len(parts) > 2 else ""
165-
# Old templates had parent CURIEs in column 3, not owl:Type
166-
owl_type = raw_type if raw_type.startswith("owl:") else "owl:Class"
163+
# Old templates had parent CURIEs in column 3, not owl:Type.
164+
# Return None so collect_labels can preserve the extract's type.
165+
owl_type = raw_type if raw_type.startswith("owl:") else None
167166
if label:
168167
labels[numeric_id] = (label, owl_type, f"tag:{tag}")
169168
return labels
@@ -177,14 +176,15 @@ def collect_labels(repo_root: Path) -> dict[str, tuple[str, str, str]]:
177176
"""
178177
extracts_dir = repo_root / "metadata/ontology/historical_submissions/entity_extracts"
179178
labels = _labels_from_extracts(extracts_dir)
180-
# Tag labels take priority, but only update type if the tag has a valid owl: type
179+
# Tag labels take priority, but only update type if the tag provides a real owl: type
181180
for numeric_id, (label, owl_type, source) in _labels_from_tags(repo_root).items():
182-
if numeric_id in labels and not owl_type.startswith("owl:"):
181+
if numeric_id in labels and owl_type is None:
183182
# Keep the extract's type, just update label and source
184183
_, existing_type, _ = labels[numeric_id]
185184
labels[numeric_id] = (label, existing_type, source)
186185
else:
187-
labels[numeric_id] = (label, owl_type, source)
186+
resolved_type = owl_type if owl_type is not None else "owl:Class"
187+
labels[numeric_id] = (label, resolved_type, source)
188188
return labels
189189

190190

@@ -209,8 +209,8 @@ def classify_reason(numeric_id: str) -> str:
209209
# Test ID
210210
if numeric_id == "9999999":
211211
return REASON_PLACEHOLDER_REMOVED
212-
# Era 3 literature-mining terms (1000001-1000327 range, retired in sub 9)
213-
if 1000001 <= n <= 1000058:
212+
# Era 3 literature-mining terms (1000001-1000327, retired in sub 9)
213+
if 1000001 <= n <= 1000327:
214214
return REASON_OUT_OF_SCOPE
215215
# Properties removed without deprecation (PR #317)
216216
if 2000200 <= n <= 2000299:

src/ontology/metpo.Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ diff-release:
7272
# Generated from historical BioPortal submissions + tagged releases.
7373
# Repo-only — not in Google Sheets. Regenerate with: make -f metpo.Makefile regenerate-deprecated
7474
../templates/deprecated.tsv: ../../metpo/scripts/generate_deprecated_template.py
75-
cd ../.. && uv run generate-deprecated-template -o $@
75+
cd ../.. && uv run generate-deprecated-template -o $(abspath $@)
7676

7777
.PHONY: regenerate-deprecated
7878
regenerate-deprecated:

0 commit comments

Comments
 (0)