-
Notifications
You must be signed in to change notification settings - Fork 162
fix: Add missing v2
constructor overrides
#3006
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
Conversation
class DataFrame(NwDataFrame[IntoDataFrameT]): | ||
_version = Version.V2 |
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.
Very important bit that I missed before 😞
It might make more sense to use what I did for Namespace
, where this is enforced at subclass-defintion-time
narwhals/narwhals/_namespace.py
Lines 172 to 186 in b4f1a47
class Namespace(Generic[CompliantNamespaceT_co]): | |
_compliant_namespace: CompliantNamespaceT_co | |
_version: ClassVar[Version] = Version.MAIN | |
def __init__(self, namespace: CompliantNamespaceT_co, /) -> None: | |
self._compliant_namespace = namespace | |
def __init_subclass__(cls, *args: Any, version: Version, **kwds: Any) -> None: | |
super().__init_subclass__(*args, **kwds) | |
if isinstance(version, Version): | |
cls._version = version | |
else: | |
msg = f"Expected {Version} but got {type(version).__name__!r}" | |
raise TypeError(msg) |
I marked as bug: incorrect result because I expect that could have some confusing consequences, so hopefully this can make the next release 🤞 |
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.
What type of PR is this? (check all applicable)
Related issues
Series.from_iterable
#2933 (comment)Checklist
If you have comments or can explain your changes, please do so below
v2
tests)