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: improve nested type docstrings #8358

Merged
merged 3 commits into from
Mar 12, 2024

Conversation

NickCrews
Copy link
Contributor

@NickCrews NickCrews commented Feb 14, 2024

The existing descriptions for these types at https://ibis-project.org/reference/expression-collections are pretty bad.

  • Improve toplevel docstring for types so you can get a sense for what they type even is.
  • Add "corresponds to" list so people
    can link this concept to concepts they are familiar to.
  • fix explanations of how the datashape is determined in .map(), .array(), and .struct().
  • some examples weren't really needed
  • used "column" not "array" when describing datashape, since "array" has another meaning in ibis

ibis/expr/types/structs.py Outdated Show resolved Hide resolved
ibis/expr/types/structs.py Outdated Show resolved Hide resolved
ibis/expr/types/structs.py Outdated Show resolved Hide resolved
ibis/expr/types/structs.py Outdated Show resolved Hide resolved
ibis/expr/types/structs.py Outdated Show resolved Hide resolved
ibis/expr/types/maps.py Show resolved Hide resolved
ibis/expr/types/arrays.py Outdated Show resolved Hide resolved
NickCrews added a commit to NickCrews/ibis that referenced this pull request Mar 5, 2024
NickCrews added a commit to NickCrews/ibis that referenced this pull request Mar 5, 2024
gforsyth pushed a commit that referenced this pull request Mar 6, 2024
@NickCrews NickCrews force-pushed the nested-docstrings branch 2 times, most recently from 099cb38 to 53c4470 Compare March 6, 2024 19:02
@NickCrews
Copy link
Contributor Author

@cpcloud now that the datashapes and datatypes guide is in, can you take a look at this again? Maybe @gforsyth also has thoughts here (this is your reward when you are helpful in other reviews 😉 ). Maybe this isn't perfect still, but I think it is a definite improvement.

@cpcloud
Copy link
Member

cpcloud commented Mar 12, 2024

Taking another look now. Thanks for bearing with me in the slightly longer review cycle 😅

- Improve toplevel docstring for types so you can get a sense for what they type even is.
- fix explanations of how the datashape is determined in .map(), .array(), and .struct().
- some examples weren't really needed
- used "column" not "array" when describing datashape, since "array" has another meaning in ibis
@cpcloud cpcloud added the docs-preview Add this label to trigger a docs preview label Mar 12, 2024
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 again!

@cpcloud cpcloud added this to the 9.0 milestone Mar 12, 2024
@ibis-docs-bot ibis-docs-bot bot removed the docs-preview Add this label to trigger a docs preview label Mar 12, 2024
@ibis-docs-bot
Copy link

ibis-docs-bot bot commented Mar 12, 2024

@cpcloud cpcloud added the docs Documentation related issues or PRs label Mar 12, 2024
@cpcloud cpcloud enabled auto-merge (squash) March 12, 2024 11:05
@cpcloud cpcloud merged commit bc9d757 into ibis-project:main Mar 12, 2024
82 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.

3 participants