Skip to content

Add multiplication to expressions#95

Open
jinmannwong wants to merge 2 commits intodevelopfrom
feature/mul
Open

Add multiplication to expressions#95
jinmannwong wants to merge 2 commits intodevelopfrom
feature/mul

Conversation

@jinmannwong
Copy link
Copy Markdown
Contributor

Description

  • New class Mul in expressions
  • New class BinMatOp, where composition of math operations is allowed
  • Define __mul__ for RepeatString, RepeatEnumerated and RepeatInteger

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

@jinmannwong
Copy link
Copy Markdown
Contributor Author

I noticed that __div__ is defined Overloaded, is there a reason that __mul__ wasn't? I have only added it to some specific attributes here, where I think the operation makes sense.

Copy link
Copy Markdown
Contributor

@colonesej colonesej left a comment

Choose a reason for hiding this comment

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

I agree with just adding to where it makes sense. I scanned around and could not see why __mul__ is a overload. Is the division symbol anyhow meaningful for ecflow in any context? like

expr1 / expr2

would this be interpreted as a Path-like operation?

@jinmannwong
Copy link
Copy Markdown
Contributor Author

Ah, I see, this could be the case.

@jinmannwong
Copy link
Copy Markdown
Contributor Author

@corentincarton, please can this be merged when you have time, if you are ok with the changes

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