Skip to content

Making expand_mini function modular - #14

Open
yelenacox wants to merge 8 commits into
mini-weaverfrom
mini-modular
Open

Making expand_mini function modular#14
yelenacox wants to merge 8 commits into
mini-weaverfrom
mini-modular

Conversation

@yelenacox

@yelenacox yelenacox commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
  • Organizing enum counter
  • Making expand_mini function more modular
  • Ensuring mini --clear will not delete permissible_values if reachable_from is not present

Closes #12

@yelenacox
yelenacox requested a review from torstees August 17, 2026 20:29

@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.

There are a couple of comments that aren't really guaranteed to require attention, but I would like for you to look into them before merging this in. Everything else looks good!

Comment thread src/tweaver/mini.py Outdated
if has_permissible or not has_ontology:
logger.info(f"Skipping {name}. Does not require expansion")
logger.info(f"Skipping {name}. Does not require expansion.")
enum_count += 1

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.

Let's see if these counters can be organized a bit better to make it easier to follow the logic for increments.

Comment thread src/tweaver/mini.py Outdated
"permissible_values" in enum and enum["permissible_values"]
)

has_reachable = enum.get("reachable_from") or {}

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.

My gut seems to want me to believe that this logic could very likely be the same as the minus. Can you give the possibility of creating a generic function that builds those "reachable_from" structures that can be used for both minus and the other?

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