Re: [PR] fix(drill-to-detail): ensure axis label filters map to original column names [superset]

[via GitHub]

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


##########
superset/connectors/sqla/models.py:
##########
@@ -360,6 +360,14 @@ def verbose_map(self) -> dict[str, str]:
 
         return verb_map
 
+    @property
+    def reverse_verbose_map(self) -> dict[str, str]:
+        """label -> column name"""
+        rev_map = {}
+        for k, v in self.verbose_map.items():
+            rev_map[v] = k
+        return rev_map

Review Comment:
   ### Inefficient reverse map property computation <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The reverse_verbose_map property method is inefficiently recalculating the 
reverse mapping every time it's accessed by iterating through verbose_map.
   
   
   ###### Why this matters
   Since verbose_map is also a property method, frequent access to 
reverse_verbose_map will cause repeated unnecessary computations which could 
impact performance for large datasets.
   
   ###### Suggested change ∙ *Feature Preview*
   Cache the reverse_verbose_map or calculate it once when verbose_map changes:
   ```python
   def __init__(self):
       self._reverse_verbose_map = None
   
   @property 
   def verbose_map(self) -> dict[str, str]:
       verb_map = {...}
       self._reverse_verbose_map = None # Invalidate cache
       return verb_map
   
   @property
   def reverse_verbose_map(self) -> dict[str, str]:
       if self._reverse_verbose_map is None:
           self._reverse_verbose_map = {v: k for k, v in 
self.verbose_map.items()}
       return self._reverse_verbose_map
   ```
   
   
   ###### 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/e0fb7989-aed0-4041-98fe-0163f636ca0b/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0fb7989-aed0-4041-98fe-0163f636ca0b?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/e0fb7989-aed0-4041-98fe-0163f636ca0b?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/e0fb7989-aed0-4041-98fe-0163f636ca0b?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0fb7989-aed0-4041-98fe-0163f636ca0b)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:98afb300-ec62-4a48-8b4a-297a81ededea -->
   
   
   [](98afb300-ec62-4a48-8b4a-297a81ededea)



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