CI: Unpin pyarrow<10 #50314
CI: Unpin pyarrow<10 #50314
Conversation
phofl
added
CI
| with pytest.raises(TypeError, match="different 'freq'"): | ||
| pa.array(periods, type=ArrowPeriodType("T")) | ||
| else: | ||
| result = pa.array(periods, type=ArrowPeriodType("T")) |
There was a problem hiding this comment.
do you know why this started working?
There was a problem hiding this comment.
OK, it's probably better if someone familiar with pyarrow reviews this. I don't see why a Int64array is returned after passing type ArrowPeriodType, nor why the freq doesn't seem to make a difference
(Pdb) pa.array(periods, type=ArrowPeriodType("T"))
<pyarrow.lib.Int64Array object at 0x7fb99f3aab80>
[
17167,
17168,
17169
]
(Pdb) pa.array(periods, type=ArrowPeriodType("M"))
<pyarrow.lib.Int64Array object at 0x7fb99f3aaa60>
[
17167,
17168,
17169
]
(Pdb) pa.array(periods, type=ArrowPeriodType("fdafsdfas"))
<pyarrow.lib.Int64Array object at 0x7fb99f3aaa00>
[
17167,
17168,
17169
]but it's probably my lack of familiarity
There was a problem hiding this comment.
I think would still expect this to fail if converting to another freq used to fail cc @jorisvandenbossche
There was a problem hiding this comment.
Ah, that seems an oversight in a recent improvement to allow specifying extension types in pa.array(..) (so specifically when passing objects that define __arrow_array__, we ignore the user specified type ..).
(sidenote: we should really try to catch such issues before releasing .. We never tested against the nightly conda/wheel package of pyarrow?)
About failing to cast: I am not sure that is possible / easy (without adding some mechanism to let the extension type control the cast behaviour). How it works right now (once fixed) is that the input is first cast to the extension type's storage type (in this case int64), and then the extension array is constructor from that storage.
Which doesn't make sense here in the case of two different period freqs, so that's clearly a sign we need to make this "smarter" somehow.
There was a problem hiding this comment.
Thanks for the explanation. Marked test as xfail for now
|
no opinion, could do but don't have to I think |
|
Thanks @phofl |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.