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

Boolean operation in NumPy-style type annotation raises error #93

Closed
sisp opened this issue Jul 18, 2022 · 13 comments · Fixed by #109
Closed

Boolean operation in NumPy-style type annotation raises error #93

sisp opened this issue Jul 18, 2022 · 13 comments · Fixed by #109

Comments

@sisp
Copy link
Contributor

sisp commented Jul 18, 2022

Describe the bug

A NumPy-style type annotation of the form A or B (e.g. int or float) raises the following error when building MkDocs docs with mkdocstrings:

griffe: Failed to parse annotation from 'BoolOp' node at ...

This issue seems to be related to #82 where the comment

it's probably wrong to use boolean operations in annotations (a and b, a or b)

-- #82 (comment)

is incorrect according to https://numpydoc.readthedocs.io/en/latest/format.html#parameters.

To Reproduce

"""Docstring with boolean operation in NumPy-style type annotation.

Parameters
----------
value : int or float
    Some int or float value.
"""

Expected behavior
This docstring should parse fine.

System:

  • griffe version: 0.22.0
  • Python version: 3.9
  • OS: Linux
@tristanlatr
Copy link

Parsing the numpy types can be quite hard because they allow free text as well as keywords, here is how we do it in pydoctor: https://github.com/twisted/pydoctor/blob/4176822d7d79e4a55bab6be229266a2c9c85d21b/pydoctor/napoleon/docstring.py#L159 the code is based on sphinx’s napoleon extension.

This code could be adapted to output markdown instead of restructuredtext.

@sisp
Copy link
Contributor Author

sisp commented Aug 5, 2022

/cc @pawamoy

@jmp75
Copy link

jmp75 commented Aug 13, 2022

I bumped into the same issue ERROR - griffe: Failed to parse annotation from 'BoolOp' node with Google-style docstrings, with argument types (str or None). It is also worth mentioning that my build on readthedocs.org was reported as successful, yet I did not see the expected changes in the result. Only by looking at the raw logs did I notice that griffe reported several errors.

I can work around this issue by using (Optional[str]) instead of str or None

@tristanlatr
Copy link

The core of the issue is to assume the numpy types as parsable with ast.parse. Because they are not. We must handle them by natural language processing.

Here are a few examples:

List[str] or list(bytes), optional
{"html", "json", "xml"}, optional
list of int or float or None, default: None
complicated string or `strIO <twisted.python.compat.NativeStringIO>`

@pawamoy
Copy link
Member

pawamoy commented Aug 17, 2022

Types that are not valid Python (syntax error) are fine indeed. The issue arises when the type is valid Python but not a valid type. Rather than adding complex logic to parse complex Numpy types, we could consider simply ignoring BoolOp errors (as to not make the build fail). If users want automatic cross-reference, they should use valid types and not natural language. What do you all think?

@sisp
Copy link
Contributor Author

sisp commented Aug 17, 2022

@pawamoy Would ignoring BoolOp errors suffice for handling some other natural language non-type cases like the following (taken from numpydoc)?

Parameters
----------
dtype : data-type
iterable : iterable object
shape : int or tuple of int
files : list of str

@pawamoy
Copy link
Member

pawamoy commented Aug 17, 2022

  • data-type: not sure, sub node probably not supported, will probably trigger a warning as well
  • iterable object: syntax error, ignored
  • int or tuple of int: syntax error, ignored
  • list of str: syntax error, ignored

@pawamoy
Copy link
Member

pawamoy commented Sep 23, 2022

I've decided to use the DEBUG level for such log messages when parsing Numpydoc docstrings. This will prevent your builds from failing on errors or warnings. Other parsers are left untouched and will continue logging errors.

@sisp
Copy link
Contributor Author

sisp commented Oct 5, 2022

Thanks, @pawamoy! 🙏 NumPy-style parameter annotations work great now! 🙂

@pawamoy
Copy link
Member

pawamoy commented Oct 5, 2022

Ha, just realized it's you @sisp 😄Nice seeing you here and on Copier's repo!

@sisp
Copy link
Contributor Author

sisp commented Oct 5, 2022

Haha, yes, it's a small world it seems. 🤣 I recognized you a while ago on Copier's repo after we had been discussing here for a bit. 😆 mkdocstrings and the projects around it are awesome by the way! 🙏

@pawamoy
Copy link
Member

pawamoy commented Oct 5, 2022

Thanks! 😄

@dekromp
Copy link

dekromp commented Oct 26, 2022

Hi all,
just adding to this that griffe (v0.22.2) still raises an error if or is used in the Returns section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants