Fix/from native update version#3515
Conversation
in tests/translate/from_native_test.py test_init_already_narwhals and test_init_already_narwhals_unstable have the exact same source code, when the former should verify behavior in a stable version. This function used to test this, but seemed to have been mistakenly edited.
from_native on a narwhals.{DataFrame,Series} would always return
the same object even if the DataFrame._version and the version
passed into from_native disagreed.
Now, if the versions mismatch we create a new narwhals object
with the appropriate version. If the versions match, we short cut
and return the same object.
761f14c to
f1cf356
Compare
| ``` | ||
| """ | ||
|
|
||
| _version: ClassVar[Version] = Version.MAIN |
There was a problem hiding this comment.
Hoping this has been an oversight and wasn't excluded for a strong reason. This change makes the DataFrame/LazyFrame api a bit more consistent internally.
There was a problem hiding this comment.
Yeah nice one, thanks @camriddell!
If you dig through the blame (I think) I added these on Series & DataFrame at the same time as the from_* constructors.
LazyFrame didn't get any new methods, so it slipped through 😅
|
@MarcoGorelli I believe the nightly failure isn't due to this change but perhaps something that needs to be accounted for in pandas. Shall I open up an Issue with this? Or do you believe it is relevant to this PR? |
| pl_frame = pl.DataFrame({"a": [1, 2, 3]}).lazy() | ||
| nw_frame = nw_v1.from_native(pl_frame) | ||
| with pytest.raises(AttributeError): | ||
| with pytest.raises(AssertionError): |
There was a problem hiding this comment.
IIRC, there may be a comment somewhere mentioning the error?
(Ignore me if this is too vague 😂)
|
@camriddell I should probably check what was mentioned on discord - but from reading the PR I'm a bit confused on the problem that's being solved. (#3515 (comment)) is all good - but something seems strange to me that calling FWIW, I don't think |
Description
Calling
from_native(nw.DataFrame(…), version=…)would always return the passed DataFrame/Series object.This ignores the version that is specified in the version argument.
Instead, for a given narwhals DataFrame/Series
from_nativenow returns the same object if the versions match and creates a new object if the versions do not match.Note, 2 clean up items are also added in this PR.
Before:
After:
reported by hoxbro (Holoviews) on Discord #help channel
What type of PR is this? (check all applicable)
Related issues
Checklist