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

Added a context menu to allow setting final and each modifiers #11019

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

adeas31
Copy link
Member

@adeas31 adeas31 commented Aug 1, 2023

Related Issues

#5395, #5405, #5489, #5737, #7750 and #10226

Purpose

Allow setting final and each modifiers.

Approach

Added a context menu next to parameter fields with final and each options.

Things todo,

  • Update the code to set the modifiers of the element with single call to setElementModifierValue. Right now we call setElementModifierValue for each modifier.
  • Show each option only if the parameter is an array.
  • Add the same functionality for sub modifiers as well.

@adeas31 adeas31 added the COMP/GUI/OMEdit Issue and pull request related to OMEdit label Aug 1, 2023
@adeas31 adeas31 self-assigned this Aug 1, 2023
@casella
Copy link
Contributor

casella commented Sep 5, 2023

@adeas31 it would be really nice to get this into 1.22.0. Do you think you can make it?

Also, it would be really nice if we could prevent people from incorrectly setting scalar values for array modifiers. This could mean:

  • adding the dimension of the parameter to its name in the parameter dialog, e.g. w0[3] instead of w0 for the initial value of the angular speed in multibody systems
  • refusing to accept a scalar value for arrays, with and error message popping up such as "This is an array parameter. Either provide an array of values of the same dimension or set 'each' if you want to provide a single scalar value to all the elements of the array"

@adeas31
Copy link
Member Author

adeas31 commented Sep 5, 2023

@adeas31 it would be really nice to get this into 1.22.0. Do you think you can make it?

1.22 might not be possible but I will try. I started working on it but then got some other work to finish. I will continue on this asap.

@casella
Copy link
Contributor

casella commented Sep 6, 2023

Maybe we should prioritize the really quick fixes. I opened a project for Release 1.22.0 with some collected issues, maybe you can have a look at them. We could branch off 1.22.0 maintenance next week, and then cherry pick the fixes for a couple more weeks or so.

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2023

CLA assistant check
All committers have signed the CLA.

Fixes OpenModelica#5395, Fixes OpenModelica#5405, Fixes OpenModelica#5489, Fixes OpenModelica#5737, Fixes OpenModelica#7750 and Fixes OpenModelica#10226

Apply the modifiers with one call of `setElementModifierValue`.
Added a new button for final and each context menu.
Improved the handling of modifiers.
@adeas31 adeas31 marked this pull request as ready for review November 2, 2023 12:51
@adeas31 adeas31 changed the title Added a context menu to allow setting final and each for parameters Added a context menu to allow setting final and each modifiers Nov 2, 2023
@adeas31 adeas31 enabled auto-merge (squash) November 2, 2023 12:54
@adeas31
Copy link
Member Author

adeas31 commented Nov 2, 2023

@adeas31 it would be really nice to get this into 1.22.0. Do you think you can make it?

A lot of changes are done so I recommend not to add this to 1.22.0. We should test this stuff thoroughly.

Also, it would be really nice if we could prevent people from incorrectly setting scalar values for array modifiers. This could mean:

  • adding the dimension of the parameter to its name in the parameter dialog, e.g. w0[3] instead of w0 for the initial value of the angular speed in multibody systems
  • refusing to accept a scalar value for arrays, with and error message popping up such as "This is an array parameter. Either provide an array of values of the same dimension or set 'each' if you want to provide a single scalar value to all the elements of the array"

Please make a separate tickets about it.

@adeas31 adeas31 merged commit 0281e54 into OpenModelica:master Nov 2, 2023
2 checks passed
@adeas31 adeas31 deleted the final-each branch November 2, 2023 13:33
@casella
Copy link
Contributor

casella commented Nov 2, 2023

Great! 😃

@casella
Copy link
Contributor

casella commented Nov 2, 2023

@adeas31 it would be really nice to get this into 1.22.0. Do you think you can make it?

A lot of changes are done so I recommend not to add this to 1.22.0. We should test this stuff thoroughly.

I agree, but we should have it in 1.22.1, see #5395. Then we can make a beta and test it thoroughly.

Also, it would be really nice if we could prevent people from incorrectly setting scalar values for array modifiers. This could mean:

  • adding the dimension of the parameter to its name in the parameter dialog, e.g. w0[3] instead of w0 for the initial value of the angular speed in multibody systems
  • refusing to accept a scalar value for arrays, with and error message popping up such as "This is an array parameter. Either provide an array of values of the same dimension or set 'each' if you want to provide a single scalar value to all the elements of the array"

Please make a separate tickets about it.

Will do

adeas31 added a commit to adeas31/OpenModelica that referenced this pull request Nov 14, 2023
…odelica#11019)

Fixes OpenModelica#5395, Fixes OpenModelica#5405, Fixes OpenModelica#5489, Fixes OpenModelica#5737, Fixes OpenModelica#7750 and Fixes OpenModelica#10226

Apply the modifiers with one call of `setElementModifierValue`.
Added a new button for final and each context menu.
Improved the handling of modifiers.
adeas31 added a commit that referenced this pull request Nov 14, 2023
Fixes #5395, Fixes #5405, Fixes #5489, Fixes #5737, Fixes #7750 and Fixes #10226

Apply the modifiers with one call of `setElementModifierValue`.
Added a new button for final and each context menu.
Improved the handling of modifiers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP/GUI/OMEdit Issue and pull request related to OMEdit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants