Re: [PR] fix: Makes time compare migration more resilient [superset]

Tue, 27 May 2025 08:25:47 -0700 


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


##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -296,7 +296,9 @@ def _pre_action(self) -> None:
 
         if time_compare := self.data.get("time_compare"):
             self.data["time_compare"] = [
-                value + " ago" for value in as_list(time_compare) if value
+                value if value.endswith(" ago") else value + " ago"
+                for value in as_list(time_compare)
+                if value

Review Comment:
   ### Inefficient String Operations in List Comprehension <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   List comprehension creates a new list and performs string operations for 
each item when checking and appending strings.
   
   
   ###### Why this matters
   For large time_compare lists, this creates unnecessary temporary string 
objects and performs redundant string operations. It also has to traverse the 
input list twice - once for the endswith check and once for the string 
concatenation.
   
   ###### Suggested change ∙ *Feature Preview*
   Combine the string operations into a single pass using a generator 
expression with a helper function:
   ```python
   def append_ago(value: str) -> str:
       return value if value.endswith(" ago") else f"{value} ago"
   
   self.data["time_compare"] = [append_ago(value) for value in 
as_list(time_compare) if 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/970ea02d-7b56-4771-a227-f0e84065620b/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/970ea02d-7b56-4771-a227-f0e84065620b?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/970ea02d-7b56-4771-a227-f0e84065620b?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/970ea02d-7b56-4771-a227-f0e84065620b?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/970ea02d-7b56-4771-a227-f0e84065620b)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:cb921558-1ac9-4271-a088-dbc84b9fd81c -->
   
   
   [](cb921558-1ac9-4271-a088-dbc84b9fd81c)



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