-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: require Index objects earlier in internals #33100
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
Changes from all commits
9b38dc5
d8a7221
9ab2987
7dba715
70573e8
0171bee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
import itertools | ||
import operator | ||
import re | ||
from typing import Dict, List, Optional, Sequence, Tuple, TypeVar, Union | ||
from typing import DefaultDict, Dict, List, Optional, Sequence, Tuple, TypeVar, Union | ||
import warnings | ||
|
||
import numpy as np | ||
|
@@ -342,7 +342,7 @@ def _verify_integrity(self) -> None: | |
tot_items = sum(len(x.mgr_locs) for x in self.blocks) | ||
for block in self.blocks: | ||
if block._verify_integrity and block.shape[1:] != mgr_shape[1:]: | ||
construction_error(tot_items, block.shape[1:], self.axes) | ||
raise construction_error(tot_items, block.shape[1:], self.axes) | ||
if len(self.items) != tot_items: | ||
raise AssertionError( | ||
"Number of manager items must equal union of " | ||
|
@@ -1649,7 +1649,7 @@ def concat( | |
# Constructor Helpers | ||
|
||
|
||
def create_block_manager_from_blocks(blocks, axes): | ||
def create_block_manager_from_blocks(blocks, axes: List[Index]) -> BlockManager: | ||
try: | ||
if len(blocks) == 1 and not isinstance(blocks[0], Block): | ||
# if blocks[0] is of length 0, return empty blocks | ||
|
@@ -1670,18 +1670,23 @@ def create_block_manager_from_blocks(blocks, axes): | |
except ValueError as e: | ||
blocks = [getattr(b, "values", b) for b in blocks] | ||
tot_items = sum(b.shape[0] for b in blocks) | ||
construction_error(tot_items, blocks[0].shape[1:], axes, e) | ||
raise construction_error(tot_items, blocks[0].shape[1:], axes, e) | ||
|
||
|
||
def create_block_manager_from_arrays(arrays, names, axes): | ||
def create_block_manager_from_arrays( | ||
arrays, names: Index, axes: List[Index] | ||
) -> BlockManager: | ||
assert isinstance(names, Index) | ||
assert isinstance(axes, list) | ||
assert all(isinstance(x, Index) for x in axes) | ||
|
||
try: | ||
blocks = form_blocks(arrays, names, axes) | ||
mgr = BlockManager(blocks, axes) | ||
mgr._consolidate_inplace() | ||
return mgr | ||
except ValueError as e: | ||
construction_error(len(arrays), arrays[0].shape, axes, e) | ||
raise construction_error(len(arrays), arrays[0].shape, axes, e) | ||
|
||
|
||
def construction_error(tot_items, block_shape, axes, e=None): | ||
|
@@ -1696,23 +1701,25 @@ def construction_error(tot_items, block_shape, axes, e=None): | |
if len(implied) <= 2: | ||
implied = implied[::-1] | ||
|
||
# We return the exception object instead of raising it so that we | ||
# can raise it in the caller; mypy plays better with that | ||
if passed == implied and e is not None: | ||
raise e | ||
return e | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than do this I think would be better to just mark this function with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i recall that fails on our min version build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. elsewhere (in particular in indexes.base) we've moved towards this pattern |
||
if block_shape[0] == 0: | ||
raise ValueError("Empty data passed with indices specified.") | ||
raise ValueError(f"Shape of passed values is {passed}, indices imply {implied}") | ||
return ValueError("Empty data passed with indices specified.") | ||
return ValueError(f"Shape of passed values is {passed}, indices imply {implied}") | ||
|
||
|
||
# ----------------------------------------------------------------------- | ||
|
||
|
||
def form_blocks(arrays, names, axes): | ||
def form_blocks(arrays, names: Index, axes) -> List[Block]: | ||
# put "leftover" items in float bucket, where else? | ||
# generalize? | ||
items_dict = defaultdict(list) | ||
items_dict: DefaultDict[str, List] = defaultdict(list) | ||
extra_locs = [] | ||
|
||
names_idx = ensure_index(names) | ||
names_idx = names | ||
if names_idx.equals(axes[0]): | ||
names_indexer = np.arange(len(names_idx)) | ||
else: | ||
|
@@ -1730,7 +1737,7 @@ def form_blocks(arrays, names, axes): | |
block_type = get_block_type(v) | ||
items_dict[block_type.__name__].append((i, k, v)) | ||
|
||
blocks = [] | ||
blocks: List[Block] = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as FYI but newer versions of mypy technically have better inferencing capabilities for empty sequence assignment like this. @simonjayhawkins There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. id be happy to remove this; is that the suggested course of action? |
||
if len(items_dict["FloatBlock"]): | ||
float_blocks = _multi_blockify(items_dict["FloatBlock"]) | ||
blocks.extend(float_blocks) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory mypy will flag violations here already for us, so superfluous to have these here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonjayhawkins is this correct? in a previous pass I found there were annotation-defying types being passed in this function that mypy had not caught
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, @WillAyd is correct, but in practice we probably need the run-time asserts to be sure. probably better to fail than potentially produce incorrect results.
if a variable is Any or the callsite is not being checked, then we can pass the wrong type.