Skip to content

feat(properties and taxonomy): add properties and taxonomy fields#205

Closed
dusanparipovic wants to merge 8 commits into
developfrom
properties
Closed

feat(properties and taxonomy): add properties and taxonomy fields#205
dusanparipovic wants to merge 8 commits into
developfrom
properties

Conversation

@dusanparipovic

Copy link
Copy Markdown
Contributor

No description provided.

@dusanparipovic dusanparipovic changed the title feat(properties): add properties inside system class to support filtering feat(properties): add properties inside system class May 1, 2026
@dusanparipovic dusanparipovic requested review from aoustry and tbittar May 1, 2026 15:53
ConstantData,
DataBase,
PortRef,
PortsConnection,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

isn't used

Comment thread src/gems/study/system.py
model=model,
id=id,
scenario_group=scenario_group,
properties=properties or {},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

{} because of mypy

from gems.study.scenario_builder import ScenarioBuilder


def _resolve_properties_raw_to_dict(

@dusanparipovic dusanparipovic May 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tbittar what do you think about this, do we wan't to put this here or to process parsing inside GemsViewBuilder ?

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.

Keep it here

… implement corresponding unit test for YAML parsing
Comment thread src/gems/model/parsing.py Outdated

class ModelSchema(ModifiedBaseModel):
id: str
taxonomy_category: Optional[str] = None # # Optional or mandatory,open question ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@dusanparipovic @tbittar : I suggest optional for now in GemsPy ; yet mandatory in ViewBuilder.

@aoustry

aoustry commented May 5, 2026

Copy link
Copy Markdown
Collaborator

@dusanparipovic: shouldn't we move the target branch to develop/ ?

Comment thread src/gems/model/parsing.py Outdated

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

Nice ! Can you also update the doc and the changelog ?

@dusanparipovic

Copy link
Copy Markdown
Contributor Author

Nice ! Can you also update the doc and the changelog ?

Yes no problem, sorry for late response Github doesn't show me comments for some reason... @aoustry

…my category

- Introduced optional `properties` for components in `system.yml`, allowing key/value pairs that are normalized into a dictionary.
- Added optional `taxonomy-category` field for models in library YAML files, accessible via `ModelSchema.taxonomy_category`.
- Updated documentation to reflect these changes and provided examples in the user guide.
@dusanparipovic dusanparipovic changed the base branch from main to develop May 7, 2026 07:43
- Updated documentation to replace "key" with "id" in the context of component properties in `system.yml`.
- Adjusted test assertions to reflect the change from "key" to "id" for consistency with the updated documentation.
- Ensured that error messages for duplicate properties also refer to "id" instead of "key".
Comment thread docs/CHANGELOG.md
### Added

---
- **System components: `properties`** - introduces optional `properties` on components in `system.yml` (a list of `id`/`value` pairs). These are normalized into a `dict[str, str]` on the resolved `Component` (duplicate ids raise a `ValueError`).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Second sentence is directly connected to this.
Do we really want to put this parsing here or that parsing will be done inside GVB.

@dusanparipovic dusanparipovic changed the title feat(properties): add properties inside system class feat(properties and taxonomy): add properties and taxonomy fields May 13, 2026

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

@dusanparipovic @tbittar I guess that

  • the field properties should also be added to Component ? (not only ComponentSchema).
  • the field taxonomy-category should also be added to Model? (not only ModelSchema).

Base automatically changed from develop to main May 29, 2026 11:33
@dusanparipovic dusanparipovic changed the base branch from main to develop May 29, 2026 11:55
@tbittar

tbittar commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

This PR has a lot in common with #214, I have made some comments there.

For the question on whetther properties and taxonomy-category should be in Component and Model, the thing is that it is useless for GemsPy, only useful for the ViewBuilder.

But I guess, in the future, this parsing part ModelSchema -> Model will be part of a GemsCraft module, and we do not want to return different object depending of the usage context (within GemsPy / GemsViewsBuilder), so we can always return the complete object with all fields.

what do you think @aoustry @dusanparipovic ?

@tbittar

tbittar commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

This PR seems in fact included in #214 , should it be closed ?

@aoustry aoustry deleted the branch develop June 5, 2026 15:50
@aoustry aoustry closed this Jun 5, 2026
@aoustry aoustry reopened this Jun 5, 2026
@aoustry

aoustry commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

This PR seems in fact included in #214 , should it be closed ?

Indeed, I therefore close the PR.

@aoustry aoustry closed this Jun 10, 2026
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.

3 participants