[WIP] Add QUDT units CLI commands and sync functionality #111
Conversation
mfkaptan-motius
force-pushed
the
feat/qudt-units-43
branch
3 times, most recently
from
e4cfb23 to
d242912
Compare
mfkaptan-motius
force-pushed
the
feat/qudt-units-43
branch
from
d242912 to
99ae0d5
Compare
- Add dynamic unit synchronization from QUDT repository with GraphQL enum generation - Includes custom @reference directive for semantic linking. - Cleans up stale data during new sync Signed-off-by: Mustafa Kaptan <[email protected]>
Updates CLI commands and removes legacy unit definitions. BREAKING CHANGE: Replace static YAML-based units with dynamic QUDT synchronization. Signed-off-by: Mustafa Kaptan <[email protected]>
- Add unit tests - Add E2E CLI tests - Add fixtures for units sync functionality. - Includes mocking for external QUDT API calls. Signed-off-by: Mustafa Kaptan <[email protected]>
mfkaptan-motius
force-pushed
the
feat/qudt-units-43
branch
from
99ae0d5 to
19da145
Compare
Remove reading from unit_enums.graphql while composing schema BREAKING CHANGE: unit_enums.graphql is no longer read while composing schema, tests and schema loading is broken with this commit Signed-off-by: Mustafa Kaptan <[email protected]>
- Add README covering QUDT integration, CLI usage, technical implementation, and workflow examples. - Check in some of the generated units under examples - Rename existing enums to QUDT ones in examples and test data - Add documentation to the docs-gen Signed-off-by: Mustafa Kaptan <[email protected]>
mfkaptan-motius
force-pushed
the
feat/qudt-units-43
branch
from
19da145 to
54faec0
Compare
| from s2dm.tools.validators import validate_language_tag | ||
| from s2dm.units.sync import UNITS_META_FILENAME, UNITS_META_VERSION_KEY, check_latest_qudt_version, sync_qudt_units | ||
|
|
||
| S2DM_HOME = Path.home() / ".s2dm" |
There was a problem hiding this comment.
What was the idea behind that default location?
The export commands would then also be aware of that location, right?
| ) | ||
| @click.option( | ||
| "--output-dir", | ||
| "output_dir", |
| @units.command(name="sync") | ||
| @click.option( | ||
| "--version", | ||
| "version_", |
|
|
||
| import json | ||
| import re | ||
| import urllib.request |
There was a problem hiding this comment.
I vote for requests. Much easer to use imo
| ) | ||
|
|
||
| QUDT_UNITS_TTL_URL_MAIN: str = ( | ||
| "https://raw.githubusercontent.com/qudt/qudt-public-repo/main/src/main/rdf/vocab/unit/VOCAB_QUDT-UNITS-ALL.ttl" |
There was a problem hiding this comment.
duplicated string with URL template. I would try to store the github URL only once
| """ | ||
| with urllib.request.urlopen(QUDT_GITHUB_API_URL) as resp: # nosec - public metadata | ||
| data = json.loads(resp.read().decode("utf-8")) | ||
| # Tags are objects with 'name', e.g., 'v3.1.4'. Pick the first entry. |
There was a problem hiding this comment.
I don't think that assumption is correct that the first entry will always hold the latest tag, I might be wrong though
| grouped[row.quantity_kind_label].append(row) | ||
|
|
||
| written: list[Path] = [] | ||
| effective_version = version or "main" |
There was a problem hiding this comment.
the "main" literal is used too often (3 times). Maybe use it as a default value for the cli --version?
|
|
||
| try: | ||
| version_to_use = version_ or check_latest_qudt_version() | ||
| except Exception as e: # pragma: no cover - generic guard for CLI UX |
There was a problem hiding this comment.
Too broad, it should be quite clear what the urllib.request() can throw
| else: | ||
| console.print(f"[green]✓[/green] Generated {len(written)} enum files under {output_dir}") | ||
| console.print(f"Version: {version_to_use}") | ||
| except Exception as e: # pragma: no cover - generic guard for CLI UX |
There was a problem hiding this comment.
I think you noticed, but I am not a fan of generic Exception try catches just to render a message but hiding the stacktrace in return
| try: | ||
| written = sync_qudt_units(output_dir, version_to_use, dry_run=dry_run) | ||
| if dry_run: | ||
| console.print(f"[blue]ℹ[/blue] Would generate {len(written)} enum files under {output_dir}") |
There was a problem hiding this comment.
since we do that format regularily also with warnings and errors: We should write a function to print a nice info, warning and error instead of copy pasting it everywhere
Resolves #43 #62