CoW: Return read-only array in Index.values #53704
Conversation
| if using_copy_on_write: | ||
| with pytest.raises(ValueError, match="assignment"): | ||
| idx.values[0] = 0 |
There was a problem hiding this comment.
This defeats a bit the purpose of the original test (and I don't think we need to test here that assigning into a read-only numpy array gives an error).
So maybe just remove it? (we already check np.shares_memory) Or manually set the writeable flag to True and then assign the value.
But actually, now that we keep track of references to Index data as well, the original setitem doesn't really need to do a copy, I think? (for another issue/PR)
| if not using_copy_on_write: | ||
| arr[0] = val | ||
| assert new_index[0] != val |
There was a problem hiding this comment.
Similarly here, we want to test that the copy=True keyword is honored (I assume, based on the comment). So maybe add a copy() to the line creating arr above: arr = index.values.copy(). Then arr is writeable, and the test can be done as normal (ensuring that arr was actually copied when creating the Index.
| check_names = engine != "fastparquet" | ||
| if using_copy_on_write and engine == "fastparquet": | ||
| request.node.add_marker( | ||
| pytest.mark.xfail(reason="fastparquet write into index") |
There was a problem hiding this comment.
What does fastparquet exactly? (it tries to write into the array it gets from the index? Do you know why? (that sounds as a bug in fastparquet, as it can change the dataframe you are writing?)
There was a problem hiding this comment.
I think we can simply set the flag there, they seem to use the index to do a conversion. It's already on my todo list, but don't want to block this pr because of that
jorisvandenbossche
added
Index
# Conflicts: # pandas/tests/copy_view/index/test_index.py
|
thx for noticing, rebased |
Co-authored-by: Joris Van den Bossche <[email protected]>
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.