Skip to content

Conversation

@TGooch44
Copy link
Contributor

This adds the Void and Unknown transforms that already exists in the java implementation. It's relevant to @jun-he 's open PR.


def __init__(self, source_type: Type, transform: str):
self.source_type = source_type
self.transform = transform
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to Java implementation, __str__ should return transform.

return None

def to_human_string(self, value) -> str:
return self.transform
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should return value in string format instead of transform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote this a while back and I'm actually not sure why I overrode the Transform class to_human_string, I just removed this since you're right that it wasn't doing the right thing.

@TGooch44 TGooch44 force-pushed the add_void_unknown_transform branch 2 times, most recently from 34cca87 to c8c75a4 Compare June 17, 2021 23:20
@TGooch44 TGooch44 force-pushed the add_void_unknown_transform branch from c8c75a4 to e4fbd47 Compare June 17, 2021 23:48
Copy link
Collaborator

@jun-he jun-he left a comment

Choose a reason for hiding this comment

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

LGTM!

@TGooch44 TGooch44 merged commit 765ec12 into apache:master Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants