Repository: pola-rs/polars
File: crates/polars-core/src/series/implementations/categorical.rs (Rust)

Code before:
            }

            fn vec_hash(
                &self,
                random_state: PlSeedableRandomStateQuality,
                buf: &mut Vec<u64>,
            ) -> PolarsResult<()> {
                self.0.physical().vec_hash(random_state, buf)?;
                Ok(())
            }

            fn vec_hash_combine(
                &self,
                build_hasher: PlSeedableRandomStateQuality,
                hashes: &mut [u64],
            ) -> PolarsResult<()> {
                self.0.physical().vec_hash_combine(build_hasher, hashes)?;
                Ok(())
            }

            #[cfg(feature = "algorithm_group_by")]
            unsafe fn agg_min(&self, groups: &GroupsType) -> Series {
                if self.0.uses_lexical_ordering() {
                    let s = self
                        .0
                        .cast(&DataType::String)
                        .unwrap()
                        .agg_min(groups);
                    $ca::from_str_iter(
                        s.name().clone(),
                        self.dtype().clone(),
                        s.str().unwrap().iter(),
                    )
                    .unwrap()
                    .into_series()
                } else {
                    self.apply_on_phys(|phys| phys.agg_min(groups).$ca_fn().unwrap().clone())
                        .into_series()
                }
            }

            #[cfg(feature = "algorithm_group_by")]
            unsafe fn agg_max(&self, groups: &GroupsType) -> Series {
                if self.0.uses_lexical_ordering() {
                    let s = self
                        .0
                        .cast(&DataType::String)
                        .unwrap()
                        .agg_max(groups);
                    $ca::from_str_iter(
                        s.name().clone(),

Review comment (performance):
The implementation should not cast to string here, that can end up being extremely inefficient. There should be a dedicated codepath.

It may not be a trivial implementation to make. Given that this already fixes the Enum case, it's also fine for this PR if you want to just leave an error with a message saying it's not implemented for categoricals.


Code after:
            }

            fn vec_hash(
                &self,
                random_state: PlSeedableRandomStateQuality,
                buf: &mut Vec<u64>,
            ) -> PolarsResult<()> {
                self.0.physical().vec_hash(random_state, buf)?;
                Ok(())
            }

            fn vec_hash_combine(
                &self,
                build_hasher: PlSeedableRandomStateQuality,
                hashes: &mut [u64],
            ) -> PolarsResult<()> {
                self.0.physical().vec_hash_combine(build_hasher, hashes)?;
                Ok(())
            }

            #[cfg(feature = "algorithm_group_by")]
            unsafe fn agg_min(&self, groups: &GroupsType) -> Series {
                if self.0.uses_lexical_ordering() {
                    unimplemented!()
                } else {
                    self.apply_on_phys(|phys| phys.agg_min(groups).$ca_fn().unwrap().clone())
                        .into_series()
                }
            }

            #[cfg(feature = "algorithm_group_by")]
            unsafe fn agg_max(&self, groups: &GroupsType) -> Series {
                if self.0.uses_lexical_ordering() {
                    unimplemented!()
                } else {
                    self.apply_on_phys(|phys| phys.agg_max(groups).$ca_fn().unwrap().clone())
                        .into_series()
                }
            }

            #[cfg(feature = "algorithm_group_by")]
            unsafe fn agg_list(&self, groups: &GroupsType) -> Series {
                // we cannot cast and dispatch as the inner type of the list would be incorrect
                let list = self.0.physical().agg_list(groups);
                let mut list = list.list().unwrap().clone();
                unsafe { list.to_logical(self.dtype().clone()) };
                list.into_series()
            }

            #[cfg(feature = "algorithm_group_by")]
            fn group_tuples(&self, multithreaded: bool, sorted: bool) -> PolarsResult<GroupsType> {