MNT Add ignore_types to assert_docstring_consistency
#30944
Conversation
sklearn/utils/_testing.py
Outdated
| List of parameter/attribute/return names to exclude from matching type | ||
| between objects. |
There was a problem hiding this comment.
Is this what it does? I think it would be helpful to have the example in the docstring here to be clear
| List of parameter/attribute/return names to exclude from matching type | |
| between objects. | |
| List of parameter/attribute/return names to exclude from matching type | |
| between objects. For instance, the following two would be accepted: | |
| .. code | |
| param : str, None | |
| param description | |
| param : str | |
| param description | |
There was a problem hiding this comment.
Yes that's exactly what it would do.
Do you prefer this PR to #30943 ? It is much simpler and I think I would prefer it for this reason.
There was a problem hiding this comment.
Should I just close the other PR then...or wait for more opinions? I am happy to close.
|
I'm fine with the proposal here. |
|
I think this is preferred over #30943 so I will close that PR |
|
@glemaitre @adrinjalali I think you two are the only ones following this, do we think this should go in? |
| @@ -688,7 +688,12 @@ def _get_diff_msg(docstrings_grouped): | |||
|
|
|||
|
|
|||
| def _check_consistency_items( | |||
| items_docs, type_or_desc, section, n_objects, descr_regex_pattern="" | |||
| items_docs, | |||
| type_or_desc, | |||
There was a problem hiding this comment.
Can we document that the possible values for type_or_desc is "type specification" and "description"?
There was a problem hiding this comment.
Thanks this function was definitely worth a parameter docstring section.
Reference Issues/PRs
Follows from #28678 (comment)
Alternative to #30943
What does this implement/fix? Explain your changes.
ignore_typestakes a list of param/attr/returns to exclude from matching types.Much simpler usage and implementation vs #30943 but no matching is done at all.
Any other comments?
Draft as just to show what this option would look like.
cc @glemaitre @adrinjalali @StefanieSenger