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

Support for type annotations on non-local object properties of a Class #331

Closed
etaham opened this issue Mar 18, 2024 · 7 comments
Closed

Comments

@etaham
Copy link

etaham commented Mar 18, 2024

[Feature Request]

I'm trying to organize the contents of a class and running into an error around type annotations. Here is the code:

class BugReport {
  regularVar: String = ""

  internalObj {
    wontWork: String = regularVar
  }
}

inst = new BugReport{}

Running pkl eval bugReport.pkl -f yaml yields the following error:

–– Pkl Error ––
A non-local object property cannot have a type annotation.

5 | wontWork: String = regularVar
              ^^^^^^
at bugReport#BugReport.internalObj.wontWork (file:///.../pkl/bugReport/bugReport.pkl, line 5)

Something that does work is to define a new class for internalObj, but then you can't take advantage of late binding to set defaults without slightly less natural code (where the definition and defaults needs to be defined separately). This works, but notice that the default is specified outside the class definition.

class BugReport {
  regularVar: String = ""

  internalObj: InternalObj = new InternalObj{
    willWork = regularVar
  }
}

class InternalObj {
  willWork: String
}

inst = new BugReport{}

Ask would be to make the first code snippet work.

Thank you!
Eli

@holzensp
Copy link
Contributor

One problem here is that your first code snippet defines an implicit class for internalObject. What would you expect reflect.Class(new BugReport {}.internalObj.getClass()).name to come out as?

@etaham
Copy link
Author

etaham commented Mar 18, 2024

good point - how does reflection work if not using type annotations?

class BugReport {
  regularVar: String = ""

  internalObj {
    willWork = regularVar
  }
}

Could the same be done for internal classes? If not, could it just generate "random" classname for the internal class?

@bioball
Copy link
Contributor

bioball commented Mar 18, 2024

We don't have any plans on supporting type annotations on non-local object properties.

One feature that might help you is anonymous classes; e.g.

class Something {
  foo: class {
    bar: String
  }
}

It's still unclear if we'd ever support this, but there's some valid use-cases for it. For example, for easier interop with external schema definitions (e.g. JSON Schema)

@odenix
Copy link
Contributor

odenix commented Mar 19, 2024

@etaham internalObj: InternalObj = new InternalObj{ can be simplified to internalObj: InternalObj = new {.

@etaham
Copy link
Author

etaham commented Mar 19, 2024

I'm trying to balance two goals:

  1. Every field has a type annotation and optional default (that can reference variables in the outer class)
  2. Organize fields into groups to make it simpler for folks to understand groups of parameters.

Does this make sense? How are others thinking about this?

@odenix
Copy link
Contributor

odenix commented Mar 19, 2024

I think you've already found the right solution.
If BugReport is an important class that has many "groups", i.e., smaller classes, you can turn it into its own module:

BugReport.pkl

module foo.bar.BugReport

regularVar: String?

internalObj: InternalObj = new {
  willWork = regularVar
}

class InternalObj {
  willWork: String
}

@etaham
Copy link
Author

etaham commented Mar 19, 2024

That works - it's just a little inconsistent. regularVar gets its type annotation and default in one spot, while willWork will be split.

Thank you!

@bioball bioball closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2024
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

4 participants