jroof88 commented on a change in pull request #29720:
URL: https://github.com/apache/spark/pull/29720#discussion_r488745944
##########
File path: python/pyspark/sql/types.py
##########
@@ -305,7 +305,7 @@ def jsonValue(self):
@classmethod
def fromJson(cls, json):
Review comment:
@HyukjinKwon I think going down the rabbit hole of use case is a bad
approach. Taking a step back from the use case, the motivation behind this
change is for a **consistent API when creating DataTypes** (namely
`StructField`, `ArrayType`, & `MapType`). It seems a bit inconsistent that I
can do:
```python
>> MapType(StringType(), StringType()) # notice I omit the valueContainsNull
argument
MapType(StringType,StringType,true) # valueContainsNull has a default value
of True
```
But when using `.fromJson` which is essentially a constructor for `MapType`,
with an equivalent JSON:
```python
>> MapType.fromJson({"keyType": "string", "valueType": "string"}) # No
valueContainsNull argument
KeyError: 'valueContainsNull' # Error thrown tell me I need it in my JSON
```
The same inconsistencies hold true for `StructField` keys `metadata` &
`nullable` **_AND_** `ArrayType` key `containsNull`.
On the flip side, lets say I omit the required parameter `valueType`:
```python
>> MapType(StringType()) # notice I omit the valueType parameter
TypeError: __init__() missing 1 required positional argument: 'valueType' #
Error throw telling me I need valueType
```
And when I do the same using a JSON constructing:
```
>> MapType({"keyType": "string"}) # omit the valueType parameter
KeyError: 'valueType' # Error throw telling me I need it in my JSON
``
This change proposes making the constructors, both the Python class
constructor and the classMethod `.fromJson`, consistent on what they expect for
input.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
