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

Merged
merged 8 commits into from
Mar 6, 2025

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.

Note: in ActionDslLoader.groovy, the GroovyScriptExtension found in the classpath are now loaded with the provided map of context objects. This map will be empty if not provided (and therefore it will not change anything for the users).

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]>
Signed-off-by: Nicolas Rol <[email protected]>
@marifunf
Copy link
Contributor

something similar must be done for ActionDslLoader

…make use of contextObjects

Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
@olperr1 olperr1 merged commit 49f06b5 into main Mar 6, 2025
8 checks passed
@olperr1 olperr1 deleted the nro/groovy_scripting_extension branch March 6, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants