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

docs: include return type in Interval component properties #8193

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

NickCrews
Copy link
Contributor

Fixes #8165

@cpcloud
Copy link
Member

cpcloud commented Feb 5, 2024

@NickCrews Thanks for the PR. Is there any chance that instead of this approach we can add these methods and properties individually to _quarto.yml (likely requiring dynamic: true for each of them)?

@NickCrews
Copy link
Contributor Author

NickCrews commented Mar 5, 2024

Per this thread, if I try

@public
class IntervalValue(Value):
    quarters: ir.IntegerValue
    """The number of quarters"""

    @property
    def quarters(self) -> ir.IntegerValue:
        return self.to_unit("Q")

it generates (note no docstring)

| Name | Description |
| --- | --- |
| [quarters](#ibis.expr.types.temporal.IntervalValue.quarters) |  |

Probably this is something to do with the metapgrogramming of Value using the class type annotations? So seems like this workaround won't work for us at all.

From reading that thread, I think I have come to the conclusion that my current implementation is the only way to do this currently.

@NickCrews
Copy link
Contributor Author

I think this is still a gradual improvement, even if it isn't ideal. I think it shouldn't be bad to reverse or improve this later if this functionality changes. At the least, it adds __add__ documentation, which was missing.

docs/_quarto.yml Outdated Show resolved Hide resolved
Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@cpcloud cpcloud added the docs Documentation related issues or PRs label Mar 12, 2024
@cpcloud cpcloud added this to the 9.0 milestone Mar 12, 2024
@cpcloud cpcloud changed the title docs: include return type in Interval.years, .months, etc docs: include return type in Interval component properties Mar 12, 2024
@cpcloud cpcloud merged commit 9b00657 into ibis-project:main Mar 12, 2024
90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related issues or PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: UX: improve Interval API
2 participants