Skip to content

REF: Refactor reference handling to the BlockManager level for iterrows #51431

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 5 commits into from
Feb 17, 2023

Conversation

phofl
Copy link
Member

@phofl phofl commented Feb 16, 2023

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

let's see if this passes everything

@@ -247,6 +247,16 @@ def _has_no_reference_block(self, blkno: int) -> bool:
"""
return not self.blocks[blkno].refs.has_reference()

def add_references(self, mgr: BaseBlockManager) -> None:
"""
Adds the references from one manager to another.
Copy link
Member

Choose a reason for hiding this comment

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

This assumes an identical block structure between both managers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I’ll add a comment explaining this

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

@jorisvandenbossche jorisvandenbossche changed the title REF: Refactor reference handling to the BlockManager level REF: Refactor reference handling to the BlockManager level for iterrows Feb 17, 2023
Comment on lines 1399 to 1400
# Argument 1 to "add_references" of "BaseArrayManager" has incompatible
# type "Union[BlockManager, ArrayManager]"; expected "BaseArrayManager"
Copy link
Member

Choose a reason for hiding this comment

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

To be honest, this is kind of bullshit .. (and just noise to add as a comment) There is no easy way to let mypy understand such typing with a base class?

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically we could make mypy aware that using_copy_on_write() guards agains ArrayManager, but I am not that familiar with mypy :)

Removed.

@jorisvandenbossche jorisvandenbossche added this to the 2.0 milestone Feb 17, 2023
@phofl
Copy link
Member Author

phofl commented Feb 17, 2023

Merging, this was only a removed comment

@phofl phofl merged commit 6e29dbf into pandas-dev:main Feb 17, 2023
@phofl phofl deleted the cow_manager_refs branch February 17, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants