WIP: first pass at UnboundTransform#209
Conversation
|
@Fokko this isnt completely done, but would appreciate a review + some guidance on next steps once you run linting and see whats up haha |
|
No worries on the speed of this one, I know you are heads down on getting write support |
| def __init__(self, term: BoundTerm[L], transform: Transform[L, Any]): | ||
| self.term: BoundTerm[L] = term | ||
| self.transform = transform |
There was a problem hiding this comment.
| def __init__(self, term: BoundTerm[L], transform: Transform[L, Any]): | |
| self.term: BoundTerm[L] = term | |
| self.transform = transform | |
| def __init__(self, term: BoundTerm[L], transform_func: Callable[Optional[L], Optional[Any]]): | |
| self.term: BoundTerm[L] = term | |
| self.transform = transform |
| raise ValueError("some better error message") | ||
|
|
||
| else: | ||
| return BoundTransform(bound_term, self.transform) |
There was a problem hiding this comment.
I think we should actually instantiate the callable of the transform:
| return BoundTransform(bound_term, self.transform) | |
| return BoundTransform(bound_term, self.transform.transform(bound_term.ref().field.field_type)) |
|
|
||
| def eval(self, struct: StructProtocol) -> L: | ||
| """Return the value at the referenced field's position in an object that abides by the StructProtocol.""" | ||
| return self.term.eval(struct) |
There was a problem hiding this comment.
| return self.term.eval(struct) | |
| return self.transform(self.term.eval(struct)) |
|
|
||
|
|
||
| def test_cast() -> None: | ||
| cast = parser.parse("CAST(created_at as date)") |
There was a problem hiding this comment.
I would love to see some more tests here. The most important part that's missing here is converting date to a DayTransform. Other options such as year, month etc should be added as well.
|
Hey @jayceslesar thanks for working on this, and I think you're already halfway there. The most important part is to make sure that we convert |
|
@jayceslesar #198 seems to be closed; do you mind closing this PR so the backlog is cleared? |
|
@nssalian The issue hasn't been solved unfortunally. I've reopened the issue, but this PR needs a bit more TLD |
|
Thanks for clarifying @Fokko . @jayceslesar , do you want to pick this back up again and clean it up if you have the bandwidth? |
|
@Fokko @jayceslesar , I'll take a stab at this and push a new PR if that's ok? |
Resolves #198