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

Change GroovyScriptExtension.load to use a map of context objects instead of a ComputationManager to be more generic #3308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rolnico
Copy link
Member

@rolnico rolnico commented Feb 5, 2025

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?
No

What kind of change does this PR introduce?
Feature

Does this PR introduce a new Powsybl Action implying to be implemented in simulators or pypowsybl?

  • Yes, the corresponding issue is here
  • No

What is the current behavior?
GroovyScriptExtension.load expect only a ComputationManager, it is not possible to give other objects that would be used in the binding of groovy scripts.
The issue was raised with a Class implemented in powsybl-metrix could not be used in a script through powsybl-afs since the binding could not be made using the GroovyScriptExtension system.

What is the new behavior (if this is a feature change)?
GroovyScriptExtension.load now expect a Map<Class<?>, Object> instead so that each extension implementing GroovyScriptExtension can find their specific object inside.

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Classes implementing GroovyScriptExtension have to change their method void load(Binding binding, ComputationManager computationManager) to void load(Binding binding, Map<Class<?>, Object> contextObjects)
If the computationManager was used inside, it now has to be found in the map.
See for example how the LoadFlowGroovyScriptExtension changes from:

   @Override
    void load(Binding binding, ComputationManager computationManager) {
        binding.loadFlow = { Network network, LoadFlowParameters parameters = this.parameters ->
            LoadFlow.run(network, network.getVariantManager().getWorkingVariantId(), computationManager, parameters)
        }
        binding.loadflow = { Network network, LoadFlowParameters parameters = this.parameters ->
            LoadFlow.run(network, network.getVariantManager().getWorkingVariantId(), computationManager, parameters)
        }
    }

to:

    @Override
    void load(Binding binding, Map<Class<?>, Object> contextObjects) {
        if (contextObjects.keySet().contains(ComputationManager.class)) {
            ComputationManager computationManager = contextObjects.get(ComputationManager.class) as ComputationManager

            binding.loadFlow = { Network network, LoadFlowParameters parameters = this.parameters ->
                LoadFlow.run(network, network.getVariantManager().getWorkingVariantId(), computationManager, parameters)
            }
            binding.loadflow = { Network network, LoadFlowParameters parameters = this.parameters ->
                LoadFlow.run(network, network.getVariantManager().getWorkingVariantId(), computationManager, parameters)
            }
        }
    }

Other information:

…ead of a ComputationManager to be more generic

Signed-off-by: Nicolas Rol <[email protected]>
@rolnico rolnico self-assigned this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant