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

Review and discuss Polymorphic Labels proposal #4482

Closed
onelson opened this issue Feb 15, 2022 · 2 comments
Closed

Review and discuss Polymorphic Labels proposal #4482

onelson opened this issue Feb 15, 2022 · 2 comments
Assignees

Comments

@onelson
Copy link
Contributor

onelson commented Feb 15, 2022

This ticket is to make sure the team reviews #4388 in enough depth so as to discuss it and ultimately decide next steps

DOD:

  • Get proposal accepted by team
  • Write up issues for implementing the accepted proposal
@Marwes
Copy link
Contributor

Marwes commented Feb 23, 2022

Added an RFC of sorts to outline how I think polymorphic labels should work and the issues it faces https://github.com/influxdata/flux/blob/labels/libflux/Polymorphic_Labels.md #4388

@Marwes
Copy link
Contributor

Marwes commented Mar 22, 2022

Notes from the working group

  • Some functions have default columns (usually "_value") when no columns are specified

    • We can allow specifying default values for optional arguments in builtin type signatures to match
  • Only allow static labels as values

    • This will break for users using dynamic strings
    • Tentatively we will accept these breakages but have to be careful when releasing
    • There are possibly a way to allow dynamic strings by introducing a "dynamic field" when
      a string type is used in place of a field but a record with a "dynamic field" ends up
      polluting any later uses of the record as they will default to going through the "dynamic field"
  • Allow for the .abc syntax in addition?

    • Doesn't have any specific advantage over the string literal syntax
    • String literal syntax seems required for backwards compatibility and necessary
  • Preventing breakages when releasing

    • Label inference causes changes in type inference that can break existing uses
      • Feature flagging such that label types are only introduced when the flag is on should
        prevent accidental changes
    • Changes to the type signature of existing functions may break existing uses
      • We can introduce separate, experimental equivalents and rewrite the AST to use
        those when feature flagged
      • We can introduce alternate builtin signatures which are enabled on feature flag
  • When should label types be used vs string types

    • Functions like add : (A, A) => A will want to work with strings while fill (and others that use labels) want to treat string literals as labels. We need a way to distinguish when a literal is typed as string or label. Checking if a variable has a Label kind may be enough.
    • Another solution may be to make string literals be typed as a variable with the Label kind (similiar to an IntOrFloat kind attempt for integer literals in the past). Then replace variables with unknown types with strings after inference is done.

@Marwes Marwes self-assigned this Mar 23, 2022
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

No branches or pull requests

3 participants