Skip to content
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

fix(clickhouse): more explicitly disallow null structs #9305

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

NickCrews
Copy link
Contributor

@NickCrews NickCrews commented Jun 4, 2024

I remove the docstring because it is obvious from the type signature,
and the docstring was wrong. The superclass has the correct docstring though,
just inherit that.

Part of #8666

I remove the docstring because it is obvious from the type signature,
and the docstring was wrong. The superclass has the correct docstring though,
just inherit that.
@NickCrews NickCrews force-pushed the clickhouse-nullable-nested branch from 6935974 to f930668 Compare June 4, 2024 05:59
@NickCrews NickCrews marked this pull request as ready for review June 4, 2024 06:17
@NickCrews NickCrews changed the title fix(clickhouse): handle nullable nested types fix(clickhouse): more explicitly disallow null structs Jun 4, 2024
@NickCrews NickCrews requested a review from cpcloud June 4, 2024 06:19
@cpcloud
Copy link
Member

cpcloud commented Jun 4, 2024

FYI docstrings are not inherited for overridden methods (though IPython helps us out here), but I still don't think we need it here :)

In [1]: class Base:
   ...:     def run(self):
   ...:         """Run some code"""
   ...:

In [2]: class Child(Base):
   ...:     def run(self): ...
   ...:

In [3]: Child.run?
Signature: Child.run(self)
Docstring: Run some code
File:      ~/src/github.com/ibis/<ipython-input-2-114bdfe3357a>
Type:      function

In [4]: Child.run.__doc__

In [5]: Base.run.__doc__
Out[5]: 'Run some code'

@cpcloud cpcloud added this to the 9.1 milestone Jun 4, 2024
@cpcloud cpcloud added bug Incorrect behavior inside of ibis clickhouse The ClickHouse backend labels Jun 4, 2024
@cpcloud cpcloud merged commit fc1d00f into ibis-project:main Jun 4, 2024
79 checks passed
@cpcloud
Copy link
Member

cpcloud commented Jun 4, 2024

Crushing it!

@NickCrews
Copy link
Contributor Author

ahh, thanks for the point re docstrings. VSCode is doing the same thing as ipython and shows the parent class's docstring when I hover over the method, but I see now that is being misleading as to what is actually happening :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior inside of ibis clickhouse The ClickHouse backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants