You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/18 00:26:05 UTC

[GitHub] [iceberg] kbendick opened a new pull request, #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

kbendick opened a new pull request, #4797:
URL: https://github.com/apache/iceberg/pull/4797

   Tox tests for python 3.8 are failing, as the `StructType` is now a `dataclass`, but its parent class is not a dataclass and it has `init=False`.
   
   To get around this, I've called `super().__init__()` in the post init method, as suggested here: https://stackoverflow.com/questions/66827839/super-classs-init-is-not-called-using-dataclass
   
   It's also possible we want to just set an ignore on this.
   
   cc @Fokko @samredai 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] kbendick commented on a diff in pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#discussion_r875372781


##########
python/src/iceberg/types.py:
##########
@@ -208,6 +208,10 @@ def __init__(self, *fields: NestedField, **kwargs):
             fields = kwargs["fields"]
         object.__setattr__(self, "fields", fields)
 
+    # This __post_init__ call is needed as IcebergType is not a dataclass
+    def __post_init__(self):
+        super().__init__()

Review Comment:
   Yes, I believe that interferes with setting this to a frozen dataclass.
   
   See this SO (the answer at the bottom): https://stackoverflow.com/questions/66827839/super-classs-init-is-not-called-using-dataclass
   
   As well as this example: https://github.com/ericvsmith/dataclasses/issues/142



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#issuecomment-1129871110

   Thanks for fixing this @kbendick


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] samredai commented on a diff in pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
samredai commented on code in PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#discussion_r875371866


##########
python/src/iceberg/types.py:
##########
@@ -208,6 +208,10 @@ def __init__(self, *fields: NestedField, **kwargs):
             fields = kwargs["fields"]
         object.__setattr__(self, "fields", fields)
 
+    # This __post_init__ call is needed as IcebergType is not a dataclass
+    def __post_init__(self):
+        super().__init__()

Review Comment:
   What happens if we call this in the init method right above? Does that interfere with setting this to a frozen dataclass?



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] kbendick commented on a diff in pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#discussion_r875373103


##########
python/src/iceberg/types.py:
##########
@@ -208,6 +208,10 @@ def __init__(self, *fields: NestedField, **kwargs):
             fields = kwargs["fields"]
         object.__setattr__(self, "fields", fields)
 
+    # This __post_init__ call is needed as IcebergType is not a dataclass
+    def __post_init__(self):
+        super().__init__()

Review Comment:
   To be honest, I'm not actually sure. I _think_ we might be able to safely just ignore this, but this was the suggested route so I went with it.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] a-recknagel commented on a diff in pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by "a-recknagel (via GitHub)" <gi...@apache.org>.
a-recknagel commented on code in PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#discussion_r1151809061


##########
python/src/iceberg/types.py:
##########
@@ -208,6 +208,10 @@ def __init__(self, *fields: NestedField, **kwargs):
             fields = kwargs["fields"]
         object.__setattr__(self, "fields", fields)
 
+    # This __post_init__ call is needed as IcebergType is not a dataclass
+    def __post_init__(self):
+        super().__init__()

Review Comment:
   Just in case it ever does come up, `IcebergType ` can be protected from having frozen-dataclass-children by using `object.__setattr__` for assignments instead of `=`, e.g. instead of `self.x = x` do `object.__setattr__(self, 'x', x)`.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] kbendick commented on a diff in pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#discussion_r875376330


##########
python/src/iceberg/types.py:
##########
@@ -208,6 +208,10 @@ def __init__(self, *fields: NestedField, **kwargs):
             fields = kwargs["fields"]
         object.__setattr__(self, "fields", fields)
 
+    # This __post_init__ call is needed as IcebergType is not a dataclass
+    def __post_init__(self):
+        super().__init__()

Review Comment:
   For now, let's just add a single ignore statement and then come back to this so that CI is passing in the master branch.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] kbendick commented on pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4797:
URL: https://github.com/apache/iceberg/pull/4797#issuecomment-1129443952

   Here's some example breaking output:
   ```
    linters run-test: commands[3] | pylint src tests
     ************* Module iceberg.types
     src/iceberg/types.py:206:4: W0231: __init__ method from base class 'IcebergType' is not called (super-init-not-called)
   ```


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] danielcweeks merged pull request #4797: Python - Fix tox errors for not calling super __init__ on a dataclass

Posted by GitBox <gi...@apache.org>.
danielcweeks merged PR #4797:
URL: https://github.com/apache/iceberg/pull/4797


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org