CLN: rename get_block_values, simplify #32521
Conversation
| This is used in the JSON C code. | ||
| """ | ||
| return self.get_values(dtype=dtype) | ||
| # TODO(2DEA): reshape will be unnecessary with 2D EAs |
There was a problem hiding this comment.
Putting such comments in several places won't make this happen ... (if you want this, you will need to lead a discussion about it)
There was a problem hiding this comment.
experience shows that discussion turns on amounts of complexity; marking these will allow us to pseudo-quantify that
There was a problem hiding this comment.
agree with both of you! these do have good placeholders, but am skeptical of 2D EA.
jorisvandenbossche
left a comment
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Minor comment, +1 in general on the rename
| This is used in the JSON C code | ||
| This is used in the JSON C code. | ||
| """ | ||
| return self.get_values(dtype=dtype) |
There was a problem hiding this comment.
Why not keep it as get_values() (but remove the dtype, since that is not used) ? The get_values already handles the reshape, avoiding the need to keep track of it here as well.
There was a problem hiding this comment.
Because get_values is next up on the chopping block
| This is used in the JSON C code. | ||
| """ | ||
| return self.get_values(dtype=dtype) | ||
| # TODO(2DEA): reshape will be unnecessary with 2D EAs |
There was a problem hiding this comment.
agree with both of you! these do have good placeholders, but am skeptical of 2D EA.
jreback
added
Clean
Internals
3 participants
cc @WillAyd name more explicit, implementation more transparent.