missing_indices were calculated twice in OrdinalEncoder #27011

xuefeng-xu · 2023-08-04T07:31:48Z

xuefeng-xu
Aug 4, 2023

I noticed that in OrdinalEncoder the missing_indices were calculated twice, see line 1501 and line 1512.

scikit-learn/sklearn/preprocessing/_encoders.py

Lines 1494 to 1521 in 38a06e4

    
           # `_fit` will only raise an error when `self.handle_unknown="error"` 
        
           fit_results = self._fit( 
        
               X, 
        
               handle_unknown=self.handle_unknown, 
        
               force_all_finite="allow-nan", 
        
               return_and_ignore_missing_for_infrequent=True, 
        
           ) 
        
           self._missing_indices = fit_results["missing_indices"] 
        
           cardinalities = [len(categories) for categories in self.categories_] 
        
           if self._infrequent_enabled: 
        
               # Cardinality decreases because the infrequent categories are grouped 
        
               # together 
        
               for feature_idx, infrequent in enumerate(self.infrequent_categories_): 
        
                   if infrequent is not None: 
        
                       cardinalities[feature_idx] -= len(infrequent) 
        
           # stores the missing indices per category 
        
           self._missing_indices = {} 
        
           for cat_idx, categories_for_idx in enumerate(self.categories_): 
        
               for i, cat in enumerate(categories_for_idx): 
        
                   if is_scalar_nan(cat): 
        
                       self._missing_indices[cat_idx] = i 
        
                       # missing values are not considered part of the cardinality 
        
                       # when considering unknown categories or encoded_missing_value 
        
                       cardinalities[cat_idx] -= 1 
        
                       continue

I think line 1511 to 1521 can be replaced by this below.

# missing values are not considered part of the cardinality
# when considering unknown categories or encoded_missing_value
for cat_idx, categories_for_idx in enumerate(self.categories_):
    for cat in categories_for_idx:
        if is_scalar_nan(cat):
            cardinalities[cat_idx] -= 1
            continue

I can work on this if needed.

thomasjpfan · 2023-08-04T17:31:03Z

thomasjpfan
Aug 4, 2023
Maintainer

I agree. You are welcome to open a PR with your proposed change.

1 reply

xuefeng-xu Aug 5, 2023
Author

See PR #27017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

missing_indices were calculated twice in OrdinalEncoder #27011

Uh oh!

{{title}}

Uh oh!

Replies: 1 comment 1 reply

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Uh oh!

missing_indices were calculated twice in OrdinalEncoder #27011

Uh oh!

xuefeng-xu Aug 4, 2023

Replies: 1 comment · 1 reply

Uh oh!

thomasjpfan Aug 4, 2023 Maintainer

Uh oh!

xuefeng-xu Aug 5, 2023 Author

xuefeng-xu
Aug 4, 2023

Replies: 1 comment 1 reply

thomasjpfan
Aug 4, 2023
Maintainer

xuefeng-xu Aug 5, 2023
Author