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/07/14 10:51:11 UTC

[GitHub] [iceberg] Fokko opened a new pull request, #5276: Python: Add type to the schema

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

   Since the schema is an allOf the structType, we should also inherit the type: https://github.com/apache/iceberg/blob/master/open-api/rest-catalog-open-api.yaml#L1006-L1017


-- 
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 a diff in pull request #5276: Python: Add type to the schema

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


##########
python/tests/utils/test_bin_packing.py:
##########
@@ -82,17 +81,3 @@ def weight_func(x):
         return x
 
     assert list(PackingIterator(splits, target_weight, lookback, weight_func, largest_bin_first)) == expected_lists
-
-
-def test_serialize_schema(table_schema_simple: Schema):

Review Comment:
   Ended up in the wrong file



-- 
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] rdblue commented on a diff in pull request #5276: Python: Add type to the schema

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


##########
core/src/main/java/org/apache/iceberg/SchemaParser.java:
##########
@@ -254,8 +254,8 @@ private static Types.MapType mapFromJson(JsonNode json) {
   }
 
   public static Schema fromJson(JsonNode json) {
-    Type type  = typeFromJson(json);
-    Preconditions.checkArgument(type.isNestedType() && type.asNestedType().isStructType(),
+    Type type = typeFromJson(json);
+    Preconditions.checkArgument(type != null && type.isNestedType() && type.asNestedType().isStructType(),

Review Comment:
   Could you open a separate PR for this change?



-- 
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 a diff in pull request #5276: Python: Add type to the schema

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


##########
python/tests/utils/test_bin_packing.py:
##########
@@ -82,17 +81,3 @@ def weight_func(x):
         return x
 
     assert list(PackingIterator(splits, target_weight, lookback, weight_func, largest_bin_first)) == expected_lists
-
-
-def test_serialize_schema(table_schema_simple: Schema):

Review Comment:
   Ended up in the wrong file, there is a copy in `test_schema.py`.



-- 
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 closed pull request #5276: Python: Add type to the schema

Posted by GitBox <gi...@apache.org>.
Fokko closed pull request #5276: Python: Add type to the schema
URL: https://github.com/apache/iceberg/pull/5276


-- 
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 a diff in pull request #5276: Python: Add type to the schema

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


##########
core/src/main/java/org/apache/iceberg/SchemaParser.java:
##########
@@ -254,8 +254,8 @@ private static Types.MapType mapFromJson(JsonNode json) {
   }
 
   public static Schema fromJson(JsonNode json) {
-    Type type  = typeFromJson(json);
-    Preconditions.checkArgument(type.isNestedType() && type.asNestedType().isStructType(),
+    Type type = typeFromJson(json);
+    Preconditions.checkArgument(type != null && type.isNestedType() && type.asNestedType().isStructType(),

Review Comment:
   Ah, of course. The Python changes are related, so I figured it would be okay to do it in one PR. I've opened https://github.com/apache/iceberg/pull/5291 with solely the Java change. I'll close this one and include the Python changes in the REST Catalog PR.



-- 
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