Skip to content

Conversation

merschformann
Copy link
Member

@merschformann merschformann commented Jul 10, 2025

Description

Revises the "generic" hexaly app (python-hexaly-generic) by modifying the option handling behavior.

Changes

  • All options/parameters/arguments for the python-hexaly-generic will now get relayed directly to the wrapped model.
    • This allows for much more flexibility as the naming of the parameters in the .hxm/.lsp model are freely chosen by the modeler.
    • Updated the README to explain better how to configure/run the app.
  • Allows unNest=true option for python-hexaly-generic when facing models that read from the same directory as they are located in.

Resolves ENG-6271

@merschformann merschformann marked this pull request as ready for review July 11, 2025 00:35
@merschformann merschformann requested a review from Copilot July 11, 2025 00:36
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Improves the generic Python Hexaly app to pass all CLI options directly to the wrapped model and add an unNest mode for flattening inputs, along with updated documentation.

  • Relay every provided argument as key=value to the HexalyModeler module
  • Introduce unNest=true to copy inputs/ contents into the CWD before running
  • Update README and CI scripts to reflect new usage patterns

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
python-hexaly-generic/main.py Replaced rigid nextmv.Options with generic parse_options, added unnest_directory, and now forwards all args to module.run; removed the old nextmv.write section
python-hexaly-generic/README.md Revised instructions to explain generic option syntax and unNest behavior, and updated CLI examples
python-hexaly-generic/.gitignore Added sample model and data files to git ignore
.nextmv/readme/workflow-configuration.yml Added new steps (4.sh, 5.sh) to CI configuration and marked them skipped
.nextmv/readme/python-hexaly-generic/1.sh → 5.sh scripts Updated and renumbered shell steps to match the new local, push, platform-run, and Docker workflows
Comments suppressed due to low confidence (2)

python-hexaly-generic/main.py:51

  • New logic in parse_options and unnest_directory introduces non-trivial behavior. Adding unit tests for various argument patterns and file-copy scenarios will help prevent regressions.
def parse_options() -> tuple[dict[str, str], bool]:

python-hexaly-generic/main.py:52

  • [nitpick] The docstring could be clearer about supported formats (--key=value, -key=value, key=value, standalone flags) and examples, so maintainers know exactly how CLI flags are interpreted.
    """

Copy link
Member

@sebastian-quintero sebastian-quintero left a comment

Choose a reason for hiding this comment

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

Why are we not using nextmv functions like Options, load, and stuff like that? Also the multi-file constructs?

@merschformann
Copy link
Member Author

Why are we not using nextmv functions like Options, load, and stuff like that? Also the multi-file constructs?

We can't fuse that with the model itself, as it's not written in Python. This is just a thin wrapper. Merely an args translator. I tried it in the previous version where I took more control of where the input/output comes from. However, that wasn't flexible enough for what we needed. 😞

@merschformann merschformann merged commit fdeb544 into develop Jul 18, 2025
40 checks passed
@merschformann merschformann deleted the merschformann/revise-generic-hexaly branch July 18, 2025 21:46
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