Skip to content

DOC add Bunch to public docs and API#16404

Merged
adrinjalali merged 3 commits intoscikit-learn:masterfrom
adrinjalali:doc/bunch
Feb 11, 2020
Merged

DOC add Bunch to public docs and API#16404
adrinjalali merged 3 commits intoscikit-learn:masterfrom
adrinjalali:doc/bunch

Conversation

@adrinjalali
Copy link
Copy Markdown
Member

@adrinjalali adrinjalali commented Feb 7, 2020

Adding the Bunch object to the public API in the docs, and changing the FAQ to mention it shouldn't be used as input.

ping @thomasjpfan , does this look ok?

Fixes #16390

Copy link
Copy Markdown
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @adrinjalali . We should also update the docstrings of the utilities outputting a Bunch to link to the doc now

Comment on lines +63 to +66
``Bunch`` is a dictionary-like object that exposes its keys as attributes.
It is sometimes used as an output and sometimes to store certain
information in attributes of certain objects. This object should not be
used as an input parameter.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sometimes to store certain information in attributes of certain objects

What cases is this referring to?

This object should not be used as an input parameter.

Isn't this one of these cases where we're just giving bad ideas? I.e. no one would have thought of that in the first place? Just wondering

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Isn't this one of these cases where we're just giving bad ideas? I.e. no one would have thought of that in the first place? Just wondering

It's in the FAQ, doesn't hurt to tell people not to do that.

What cases is this referring to?

named_estimator_ for instance.

@adrinjalali
Copy link
Copy Markdown
Member Author

We should also update the docstrings of the utilities outputting a Bunch to link to the doc now

That can be a good issue for a sprint.

Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

I'm +1. I would agree to merge whatever you answer you give to my comment :)


Bunch objects are sometimes used as an output for functions and methods.
They extend dictionaries by enabling values to be accessed by key,
`bunch["value_key"]`, or by an attribute, `bunch.value_key`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to follow a strict NumPy documentation (add Examples section and something about the Attributes)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Id say that's fine, the docstring is pretty clear

@adrinjalali adrinjalali merged commit 2821abc into scikit-learn:master Feb 11, 2020
@adrinjalali
Copy link
Copy Markdown
Member Author

The doc can always be improved ;)

@adrinjalali adrinjalali deleted the doc/bunch branch February 11, 2020 14:46
@NicolasHug
Copy link
Copy Markdown
Member

That can be a good issue for a sprint.

Could you open one please @adrinjalali

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020
* add Bunch to public docs and API

* address Thomas's suggestions

* use Thomas's description
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
* add Bunch to public docs and API

* address Thomas's suggestions

* use Thomas's description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

utils.Bunch's documentation.

4 participants