-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: fix the bad error raised by HDFStore.put() #38919
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 6 commits
5722e6d
ba55c20
06ee87c
99005b8
38cf514
d9f330a
6a93c8e
4778ba9
9a3ce78
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 |
---|---|---|
|
@@ -3916,6 +3916,7 @@ def get_blk_items(mgr, blocks): | |
nan_rep=nan_rep, | ||
encoding=self.encoding, | ||
errors=self.errors, | ||
block_columns=b_items, | ||
) | ||
adj_name = _maybe_adjust_name(new_name, self.version) | ||
|
||
|
@@ -4875,8 +4876,34 @@ def _unconvert_index( | |
|
||
|
||
def _maybe_convert_for_string_atom( | ||
name: str, block, existing_col, min_itemsize, nan_rep, encoding, errors | ||
name: str, | ||
block, | ||
existing_col, | ||
min_itemsize, | ||
nan_rep, | ||
encoding, | ||
errors, | ||
block_columns: List[str], | ||
): | ||
""" | ||
Ensure all elements in the given block are "string". | ||
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. ok this is more confusing, let's just remove this doc-string entirely 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. sorry english is not my first language and I don't totally understand why non-strings aren't okay to be written in HDF5. 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. if the elements are not actual strings but python objects we cann't serialize to this column type. |
||
|
||
Parameter | ||
------ | ||
name(str): | ||
block: | ||
existing_col: | ||
min_itemsize: | ||
nan_rep: | ||
encoding: | ||
errors: | ||
block_columns(list[str]): the label of columns for debug info use. | ||
|
||
Return | ||
------ | ||
Value of a block, if its dtype is object and constructed by "string" | ||
elements. | ||
""" | ||
if not block.is_object: | ||
return block.values | ||
|
||
|
@@ -4908,14 +4935,20 @@ def _maybe_convert_for_string_atom( | |
|
||
# we cannot serialize this data, so report an exception on a column | ||
# by column basis | ||
for i in range(len(block.shape[0])): | ||
|
||
# expected behaviour: | ||
# search block for a non-string object column by column | ||
for i in range(block.shape[0]): | ||
col = block.iget(i) | ||
inferred_type = lib.infer_dtype(col, skipna=False) | ||
if inferred_type != "string": | ||
iloc = block.mgr_locs.indexer[i] | ||
error_column_label = ( | ||
block_columns[i] if len(block_columns) > i else f"No.{i}" | ||
) | ||
raise TypeError( | ||
f"Cannot serialize the column [{iloc}] because\n" | ||
f"its data contents are [{inferred_type}] object dtype" | ||
f"Cannot serialize the column [{error_column_label}]\n" | ||
f"because its data contents are not [string] but " | ||
f"[{inferred_type}] object dtype" | ||
) | ||
|
||
# itemsize is the maximum length of a string (along any dimension) | ||
|
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.
this needs to be
:meth:`~HDFStore.put`
say this is an improved error message
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.
Oh thank you for that
It's morning in my timezone here and I just wake up 😅
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.
heheh, no prob this PR looks good. just a small change.