[GitHub] [superset] villebro commented on a change in pull request #17658: feat(prophet): enable confidence intervals and y_hat without forecast

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #17658:
URL: https://github.com/apache/superset/pull/17658#discussion_r764010165



##########
File path: superset/utils/pandas_postprocessing.py
##########
@@ -820,8 +820,8 @@ def prophet(  # pylint: disable=too-many-arguments
     freq = PROPHET_TIME_GRAIN_MAP[time_grain]
     # check type at runtime due to marhsmallow schema not being able to handle
     # union types
-    if not periods or periods < 0 or not isinstance(periods, int):
-        raise QueryObjectValidationError(_("Periods must be a positive integer value"))
+    if periods < 0 or not isinstance(periods, int):

Review comment:
       Should this be flipped around, just in case the user sends an empty value?
   
   ```suggestion
       if not isinstance(periods, int) or periods < 0:
   ```
   
   
   The reason we need to do this is because we're currently not validating the post transformation schemas (I've been unable to get marshmallow to work properly with intersection types). Without this change the following error will pop up:
   ![image](https://user-images.githubusercontent.com/33317356/145040355-a872c259-1335-4a7b-9b2b-6f559b954075.png)
   




-- 
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: notifications-unsubscribe@superset.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org