FIX-#7486: Add support for .astype(pandas.CategoricalDtype(…))

[camriddell]

What do these changes do?

• first commit message and PR title follow format outlined here

  NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

• passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
• passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
• signed commit with git commit -s
• Resolves BUG: .astype(mpd.CategoricalDtype(…)) fails to convert via .astype #7486
• tests added and passing: extended 2 existing tests for converting Categorical/Category data types
• module layout described at docs/development/architecture.rst is up-to-date

[sfc-gh-jkew]

This points to an underlying issue with how binary_op "eq" is handled for dtypes.

Is it maybe dtypes.py 326?

[camriddell]

Hmm, I can't tell in where in this code-path DtypesDescriptor.equals would be invoked as self.dtypes returns a pandas.Series and col_dtypes is a user-passed object. Definitely open to further discussion as I'm not terribly familiar with the code-base so there likely are mechanics that I am not aware of after a superficial skim.

---

(for now) I am still of the opinion that this is a order-induced precedence issue, where we want to use the logic defined in the modin object to see if it is equivalent to a user-passed in object.

For the case of col_dtypes == self_dtypes (the current implementation) we always delegate to col_dtypes.__eq__ to check if the two objects are equivalent. Only if this function returns NotImplemented do we invoke the self_dtypes.__eq__ method.

In the common case, col_dtypes is a built-in object (most commonly a string at this point in the code), then we can rely on the fact that strict type checking is done as part of comparison and if the types don't align the a NotImplemented is returned, thus Python then check the __eq__ method of the other object.

In the current case modin.pandas.CategoricalDtype defines an __eq__ that never returns NotImplemented (it returns False in our case) therefore the broadcasted comparison logic is never executed (as that logic exists inside of the modin.Series. In this case pandas.CategoricalDtype defines an __eq__ implementation that returns False (which is logically correct). However since we're comparing against a Series we want a Series[bool] back instead of just a single bool. This is why .all() fails as shown in the traceback.

By swapping this comparison around, we are forcing the Series.__eq__ method to be invoked which returns an object of the correct shape/results.

---

A brief snippet to describe my observations

import modin.pandas as mpd

class T:
    def __eq__(self, other):
        return False

s = mpd.Series([1, 2, 3])

print(
    f'{s == 1}',           # Series → [True, False, False]
    f'{(1).__eq__(s) = }', # NotImpelemented
    f'{1 == s}',           # Series → [True, False, False]

    f'{T() == s}',         # False (will always be False)
    f'{s == T()}',         # Series → [False, False, False]
    sep='\n{}\n'.format('-'*40)
)

[sfc-gh-mvashishtha]

I don't fully understand the root cause here, but this seems like a simple fix.

[sfc-gh-jkew]

Makes sense to me. @devin-petersohn might want to take a look.

[sfc-gh-mvashishtha]

I don't fully understand the root cause here, but this seems like a simple fix.