Re: [PR] feat(Jinja): to_datetime filter [superset]

Thu, 20 Mar 2025 14:59:38 -0700 


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


##########
superset/jinja_context.py:
##########
@@ -561,6 +561,24 @@ def __call__(
         return result
 
 
+def to_datetime(
+    value: str | None, format: str = "%Y-%m-%d %H:%M:%S"
+) -> datetime | None:
+    """
+    Parses a string into a datetime object.
+
+    :param value: the string to parse.
+    :param format: the format to parse the string with.
+    :returns: the parsed datetime object.
+    """
+    if not value:
+        return None
+
+    # This value might come from a macro that could be including wrapping 
quotes
+    value = value.strip("'\"")

Review Comment:
   ### Unclear Variable Transformation <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The intention behind stripping quotes is not immediately clear from the 
code. The comment above explains why, but the code itself could be more 
self-documenting.
   
   ###### Why this matters
   A developer would need to check the comment to understand why quotes are 
being stripped, making the code less maintainable and harder to reason about at 
first glance.
   
   ###### Suggested change ∙ *Feature Preview*
   Rename the variable to better reflect its state:
   ```python
   quoted_value = value.strip("'\"")
   return datetime.strptime(quoted_value, format)
   ```
   
   Or use a more descriptive variable name:
   ```python
   unquoted_value = value.strip("'\"")
   return datetime.strptime(unquoted_value, format)
   ```
   
   
   ###### 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/fe482acf-6996-4e4c-8460-0fdacb1bb644/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fe482acf-6996-4e4c-8460-0fdacb1bb644?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/fe482acf-6996-4e4c-8460-0fdacb1bb644?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/fe482acf-6996-4e4c-8460-0fdacb1bb644?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fe482acf-6996-4e4c-8460-0fdacb1bb644)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:1e419d53-5a2a-4554-999f-5083f34145e5 -->
   
   [](1e419d53-5a2a-4554-999f-5083f34145e5)



-- 
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: [email protected]

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