Skip to content

Mini-Weaver - #11

Open
yelenacox wants to merge 8 commits into
mainfrom
mini-weaver
Open

Mini-Weaver#11
yelenacox wants to merge 8 commits into
mainfrom
mini-weaver

Conversation

@yelenacox

Copy link
Copy Markdown
Contributor
  • Added command for inline expansion, mini
  • Looks for exclusions in the YAML file and does not include those codes in the expanded output
    • Tree:
      • minus - permissible_values - [curie]
      • minus - reachable_from - source_nodes - [curie]
  • Added function to remove permissible_values for easier re-expansion

@yelenacox
yelenacox requested a review from torstees August 14, 2026 19:18

@torstees torstees left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, and does seem to do the trick once you get the input correct. However, that expand function should probably be split up. A general rule of thumb is to have each function have a single overarching purpose. Breaking things into smaller, more specialized pieces makes your code more modular, is easier to read since the code that sits together serves a common objective, etc.

Comment thread src/tweaver/mini.py
from rich.traceback import install


def init_logging(loglevel: str | None = None):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend dropping the local init_logging and add car-utils to this and use that one instead.

Comment thread src/tweaver/mini.py
def expand_mini(
local_filepath: Path,
iri: str | None = None,
):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is doing a lot — parsing the model file, resolving enum imports, computing exclusion codes, shelling out to dragon_search per node, and writing results back to disk, all in one ~100 line function with 5+ levels of nesting. Might be worth splitting into a few smaller pieces, e.g.:

_resolve_enum_imports(model_parsed, local_filepath) — the import-filtering/glob-matching block
_compute_minus_codes(reachable) — the minus/minus_codes logic is a nice self-contained unit already
_expand_enum_for_node(node, ontology, expanded_enum, endpoint, iri) — the dragon_search subprocess call + parsed_csv handling
_write_expanded_enum(...) — the yaml dump/write at the end

That would make expand_mini read as an orchestrator (loop over imports → loop over enums → loop over nodes) rather than mixing I/O, subprocess calls, and logic together. Not blocking, just flagging since it'll get harder to touch safely as it grows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants