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

Make "inline" arithmetic work in manifests #970

Closed
2 of 4 tasks
jmcook1186 opened this issue Aug 14, 2024 · 5 comments · Fixed by #1004
Closed
2 of 4 tasks

Make "inline" arithmetic work in manifests #970

jmcook1186 opened this issue Aug 14, 2024 · 5 comments · Fixed by #1004
Assignees
Milestone

Comments

@jmcook1186
Copy link
Contributor

jmcook1186 commented Aug 14, 2024

What

Where a number is passed in a manifest, a sum should also work, e.g.

The following should be allowed:

some-param: 30*2
duration: 60*60*24*7*4

and also the following, using other parameters in the manifest:

some-param: other-param*3

Why

This reduces the number of plugins required in pipelines because simple conversions that would otherwise need dedicated instances of our generic builtins can be done inline.

Context

Part of UX improvements

Prerequisites/resources
n/a

SoW (scope of work)

  • make necessary updates to IF and builtin code
  • documentation updated
  • test cases added
  • add guidance to plugin builder docs to help plugin builders enable this new feature

Acceptance criteria

Given (Setup): Inline arithmetic is enabled for manifests

When (Action): I run the following manifest

name: coefficient-demo
description: successful path
tags:
initialize:
  plugins:
    coefficient:
      method: Coefficient
      path: "builtin"
      global-config:
        input-parameter: carbon * 3
        coefficient: 3*arbitrary-constant
        output-parameter: "carbon-product"
tree:
  children:
    child:
      pipeline:
        compute:
          - coefficient
      inputs:
        - timestamp: 2023-08-06T00:00
          duration: 3600
          carbon: 3
          arbitrary-constant: 2

Then (Assertion): I get the following output:

name: coefficient-demo
description: successful path
tags:
initialize:
  plugins:
    coefficient:
      method: Coefficient
      path: "builtin"
      global-config:
        input-parameter: carbon * 3
        coefficient: 3*arbitrary-constant
        output-parameter: "carbon-product"
tree:
  children:
    child:
      pipeline:
        compute:
          - coefficient
      inputs:
        - timestamp: 2023-08-06T00:00
          duration: 3600
          carbon: 3
          arbitrary-constant: 2
      outputs:
        - timestamp: 2023-08-06T00:00
          duration: 3600
          carbon: 3
          arbitrary-constant: 2
          carbon-product: 54 // because we are doing (3 * 3) * (3 + 2) 
@jmcook1186 jmcook1186 added this to IF Aug 14, 2024
@jmcook1186 jmcook1186 moved this to In Refinement in IF Aug 14, 2024
@manushak
Copy link
Contributor

@jmcook1186, should the inline arithmetic feature work only for multiplication, or should it also allow division, addition, and subtraction? For example, if we have coefficient: 3*arbitrary-constant, it will be hard to distinguish which part is the parameter's name and which part is the math operation (I mean this part: arbitrary-constant).

@jmcook1186
Copy link
Contributor Author

oh that's a good point.

Maybe we can include a keyword and require names to be wrapped in ' ' in order for inline arithmetic to work e.g.
e.g. the = operator is used to tell IF to evaluate the following phrase:

coefficient: = 3 * 'arbitrary-constant'

Wyt? Do you have any alternative suggestions?

@manushak

@manushak
Copy link
Contributor

yeah, that's a good idea, but in the case of duration: 60*60*24*7*4 I don't think it will look like natural. Anyway, I don't have any suggestions right now, so I'll add =

@zanete zanete moved this from In Refinement to In Progress in IF Aug 21, 2024
@manushak manushak moved this from In Progress to Pending Review in IF Sep 9, 2024
@zanete zanete linked a pull request Sep 9, 2024 that will close this issue
9 tasks
@zanete
Copy link

zanete commented Sep 9, 2024

@jmcook1186 please add the feature request in relation to this issue as part of the description

@zanete zanete moved this from Pending Review to In Progress in IF Sep 9, 2024
@zanete
Copy link

zanete commented Sep 12, 2024

need a release in if-core before creating a PR, @narekhovhannisyan please help with that 🙏

@zanete zanete moved this from In Progress to Blocked in IF Sep 12, 2024
@zanete zanete added the blocked The issue is blocked and cannot proceed. label Sep 12, 2024
@zanete zanete removed the blocked The issue is blocked and cannot proceed. label Sep 16, 2024
@zanete zanete moved this from Blocked to Pending Review in IF Sep 16, 2024
@zanete zanete moved this from Pending Review to In Progress in IF Sep 17, 2024
@zanete zanete moved this from In Progress to Testing in IF Sep 17, 2024
@zanete zanete mentioned this issue Sep 17, 2024
2 tasks
@github-project-automation github-project-automation bot moved this from Testing to Done in IF Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants