rdblue commented on pull request #3952:
URL: https://github.com/apache/iceberg/pull/3952#issuecomment-1019000279


   @CircArgs, I'm trying to understand the goal here. It looks like this makes 
the type classes both types and literals at the same time. I can now have both 
`Integer()` and `Integer(5)`. The first is a type. But the second is also a 
type, but one that is used as a literal? What is the value of mixing the two 
classes together?
   
   I probably don't understand the benefit, but I do see some drawbacks. First, 
I think that currently `Integer() == Integer()` will be `False`. That's 
confusing to me because I think those should be equivalent. Second, 
`isinstance(Integer(5), PrimitiveType)` is `True`, so the literal is reporting 
that it can be used as a type.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to