Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Node::render() to load and merge classes into the target node #15

Merged
merged 6 commits into from
Sep 11, 2023

Conversation

simu
Copy link
Member

@simu simu commented Sep 5, 2023

This PR makes use of the methods introduced in #13 and #14 to provide method Node::render() which iteratively and recursively loads all classes which are transitively included by the Node and merges the contents of those classes into the node, flattening any ValueList entries which are created by merging the parameters mappings provided in each individual class.

Additionally, this PR introduces method Node::load_class() which loads classes from Reclass::classes_path and returns them as Node structs.

The PR also updates Reclass::nodeinfo() to call Node::render().

Finally, the PR updates the classes included by node n1 in the test inventory to have some parameters which will get merged into the node's parameters, and add a new node n2 in the test inventory which uses a relative class include in a nested class.

With this PR, our implementation should match the results of Python Reclass for nodes which don't use any parameter references. Note that we don't support most of the Python Reclass configuration options yet, the only option which is implemented is ignore_class_notfound.

Checklist

  • The PR has a meaningful title. The title will be used to auto generate the changelog
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency, internal
    as they show up in the changelog
  • Link this PR to related PRs or issues.

@simu simu added the enhancement New feature or request label Sep 5, 2023
@simu simu force-pushed the feat/flatten-value branch from 984aaa9 to e1bbcea Compare September 6, 2023 11:17
@simu simu force-pushed the feat/load-classes branch 2 times, most recently from 0126f28 to 779e5b9 Compare September 6, 2023 12:10
@simu simu force-pushed the feat/flatten-value branch from e1bbcea to ff24a14 Compare September 6, 2023 17:41
@simu simu force-pushed the feat/load-classes branch 2 times, most recently from b30d9cb to 59294b4 Compare September 6, 2023 20:54
@simu simu force-pushed the feat/flatten-value branch from ff24a14 to 8bf75f9 Compare September 6, 2023 20:54
Base automatically changed from feat/flatten-value to main September 7, 2023 12:14
@simu simu force-pushed the feat/load-classes branch from 59294b4 to b644744 Compare September 7, 2023 12:15
@simu simu requested a review from a team September 7, 2023 14:00
@simu simu force-pushed the feat/load-classes branch from b644744 to cc6cbab Compare September 8, 2023 12:02
@simu simu force-pushed the feat/load-classes branch 2 times, most recently from 1b2774b to 93dc801 Compare September 8, 2023 17:17
@simu simu requested a review from bastjan September 8, 2023 17:17
@simu simu force-pushed the feat/load-classes branch 11 times, most recently from 2b7cd26 to fc05daf Compare September 8, 2023 19:59
simu added 3 commits September 8, 2023 22:36
Also update Python tests to be able to handle the exception raised by
the `Reclass` constructor, and adjust all existing tests to use the
existing test inventory unless we want to test the error handling.
We add a method which loads classes from `Reclass::classes_path` and
returns them as `Node` structs. This will be used in the method which
loads and merges included classes into a node.
We implement `Node::render()` as the rough equivalent of Python
reclass's `_node_entity()` with `render_impl()` loosely matching
`_recurse_entity()`.

We also update the classes included by node `n1` in the test inventory
to have some parameters which will get merged into the node's
parameters.
simu added 3 commits September 8, 2023 22:36
We add node `n2` in the test inventory which includes class
`nested.cls1`. That class includes `.cls2` which should resolve to
`nested.cls2`.
Also update the Python tests to reflect the new functionality.
@simu simu force-pushed the feat/load-classes branch from fc05daf to dd45fd2 Compare September 8, 2023 20:39
@simu simu merged commit 2d96266 into main Sep 11, 2023
@simu simu deleted the feat/load-classes branch September 11, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants