blcksrx commented on a change in pull request #10450:
URL: 
https://github.com/apache/incubator-superset/pull/10450#discussion_r461482622



##########
File path: superset/views/database/forms.py
##########
@@ -303,8 +300,11 @@ def at_least_one_schema_is_allowed(database: Database) -> 
bool:
         ],
     )
 
-    sheet_name = StringField(
-        _("Sheet Name"), description="Sheet Name", validators=[Optional()]
+    sheet_name = IntegerField(
+        _("Sheet Name"),
+        description="Integers used in zero-indexed sheet positions (0 is first 
sheet).",
+        validators=[Optional(), NumberRange(min=0)],
+        widget=BS3TextFieldWidget(),

Review comment:
       The `Sheet Name` could be string

##########
File path: superset/views/database/forms.py
##########
@@ -303,8 +300,11 @@ def at_least_one_schema_is_allowed(database: Database) -> 
bool:
         ],
     )
 
-    sheet_name = StringField(
-        _("Sheet Name"), description="Sheet Name", validators=[Optional()]
+    sheet_name = IntegerField(
+        _("Sheet Name"),
+        description="Integers used in zero-indexed sheet positions (0 is first 
sheet).",
+        validators=[Optional(), NumberRange(min=0)],
+        widget=BS3TextFieldWidget(),

Review comment:
       The `Sheet Name` could be string

##########
File path: superset/views/database/forms.py
##########
@@ -303,8 +300,11 @@ def at_least_one_schema_is_allowed(database: Database) -> 
bool:
         ],
     )
 
-    sheet_name = StringField(
-        _("Sheet Name"), description="Sheet Name", validators=[Optional()]
+    sheet_name = IntegerField(
+        _("Sheet Name"),
+        description="Integers used in zero-indexed sheet positions (0 is first 
sheet).",
+        validators=[Optional(), NumberRange(min=0)],
+        widget=BS3TextFieldWidget(),
     )

Review comment:
       The `sheet_name` could be a type of string

##########
File path: superset/views/database/forms.py
##########
@@ -303,8 +300,11 @@ def at_least_one_schema_is_allowed(database: Database) -> 
bool:
         ],
     )
 
-    sheet_name = StringField(
-        _("Sheet Name"), description="Sheet Name", validators=[Optional()]
+    sheet_name = IntegerField(
+        _("Sheet Name"),
+        description="Integers used in zero-indexed sheet positions (0 is first 
sheet).",
+        validators=[Optional(), NumberRange(min=0)],
+        widget=BS3TextFieldWidget(),
     )

Review comment:
       Base on the **pandas** docs the sheet_name could be `string`, `int`, 
`list` and `None`. The `StringField` is better than `IntegerField` because of 
Softwares that parse excel format like **Microsoft office** and **LibreOffice** 
save the sheets name as `string` and by default, it is `Sheet1`. In addition, 
the sheet_name want the name of the sheet!. In the case of the number like 0,1, 
etc it would be `sheet_number`.
   I totally agree to change the default value from `None` to the `0`.
   thanks




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

Reply via email to