Skip to content

CLN: remove CategoricalBlock.to_native_types #33063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2020

Conversation

jorisvandenbossche
Copy link
Member

The implementation of to_native_types on CategoricalBlock seems duplicated with the one of ExtensionBlock (the default for na_rep is different, but internally we always pass a value for that and never use the default).

@jorisvandenbossche jorisvandenbossche added Internals Related to non-user accessible pandas implementation Clean labels Mar 27, 2020
@jorisvandenbossche jorisvandenbossche added this to the 1.1 milestone Mar 27, 2020
@simonjayhawkins
Copy link
Member

The implementation of to_native_types on CategoricalBlock seems duplicated with the one of ExtensionBlock

is np.array(values, dtype="object") equivalent to values.astype(str) for Categorical?

(the default for na_rep is different, but internally we always pass a value for that and never use the default)

also could preserve the signature and call super or would we not expect CategoricalBlock to be used by downstream projects?

@simonjayhawkins
Copy link
Member

is np.array(values, dtype="object") equivalent to values.astype(str) for Categorical?

@jorisvandenbossche you can ignore that, I was on an older commit. it's now np.array(values, dtype="object") vs np.asarray(values.astype(object))

@jorisvandenbossche
Copy link
Member Author

also could preserve the signature and call super or would we not expect CategoricalBlock to be used by downstream projects?

I don't expect anybody to use to_native_types (downstream projects use blocks, but typically for construction or accessing the values block-wise).

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jorisvandenbossche lgtm

@jreback jreback merged commit c69f7d8 into pandas-dev:master Mar 27, 2020
@jreback
Copy link
Contributor

jreback commented Mar 27, 2020

thanks

@jorisvandenbossche jorisvandenbossche deleted the cat-block-clean branch March 27, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants