Re: [PR] Fixed numeric filtering issue in Superset [superset]

[via GitHub]

korbit-ai[bot] commented on code in PR #33222:
URL: https://github.com/apache/superset/pull/33222#discussion_r2056759853


##########
superset/models/helpers.py:
##########
@@ -1162,6 +1162,11 @@ def filter_values_handler(  # pylint: 
disable=too-many-arguments  # noqa: C901
         def handle_single_value(value: Optional[FilterValue]) -> 
Optional[FilterValue]:
             if operator == utils.FilterOperator.TEMPORAL_RANGE:
                 return value

Review Comment:
   ### Missing temporal range value handling explanation <sub>![category 
Documentation](https://img.shields.io/badge/Documentation-7c3aed)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The handle_single_value function's docstring is missing an explanation of 
the special handling for temporal range operators.
   
   ###### Why this matters
   Without understanding why temporal range values are returned as-is, future 
maintainers may accidentally modify this special case handling.
   
   ###### Suggested change ∙ *Feature Preview*
   def handle_single_value(value: Optional[FilterValue]) -> 
Optional[FilterValue]:
               # Return temporal range values unmodified as they require 
special handling by the caller
               if operator == utils.FilterOperator.TEMPORAL_RANGE:
                   return value
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/977581d3-b4d1-4f6f-b7d9-fa8da4f3c33b/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/977581d3-b4d1-4f6f-b7d9-fa8da4f3c33b?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/977581d3-b4d1-4f6f-b7d9-fa8da4f3c33b?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/977581d3-b4d1-4f6f-b7d9-fa8da4f3c33b?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/977581d3-b4d1-4f6f-b7d9-fa8da4f3c33b)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:bcabf10d-cf0d-47d0-aec4-f2ce41cb8662 -->
   
   
   [](bcabf10d-cf0d-47d0-aec4-f2ce41cb8662)



-- 
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-unsubscr...@superset.apache.org

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


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