mitsuba3 icon indicating copy to clipboard operation
mitsuba3 copied to clipboard

[🐛 bug report] site-packages\mitsuba\__init__.pyi:870: error: invalid syntax

Open petsuter opened this issue 1 year ago • 10 comments

Summary

The mitsuba 3.0.1 stub file contains invalid syntax.

System configuration

  • mypy version: 0.971
  • Python version: 3.9
  • Mitsuba 3 version: 3.0.1

Description

The mitsuba 3.0.1 stub file mitsuba\__init__.pyi contains this invalid syntax on line 870:

def __init__(self: mitsuba.BSDFContext, mode: mitsuba.TransportMode = <TransportMode.Radiance: 0>) -> None:

Steps to reproduce

  1. pip install mitsuba==3.0.1
  2. Create test.py:
import mitsuba
  1. mypy test.py
C:\Python39\lib\site-packages\mitsuba\__init__.pyi:870: error: invalid syntax
Found 1 error in 1 file (errors prevented further checking)

petsuter avatar Jul 29 '22 08:07 petsuter

Python version 3.10 gives a different error:

C:\Python310\lib\site-packages\mitsuba\__init__.pyi:9256: error: unmatched ')'
Found 1 error in 1 file (errors prevented further checking)

That line:

    def , has_vertex_normals: bool = False, has_vertex_texcoords: bool = False) -> None:

petsuter avatar Jul 29 '22 08:07 petsuter

I guess the BSDFContext constructor BSDFContext() = default; and the default member initializer TransportMode mode = TransportMode::Radiance; cause pybind11 to create this documentation:

>>> help(mitsuba.BSDFContext)
Help on class BSDFContext in module mitsuba:

class BSDFContext(pybind11_builtins.pybind11_object)
... |
 |  Methods defined here:
 |
 |  __init__(...)
 |      __init__(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. __init__(self: mitsuba.BSDFContext, mode: mitsuba.TransportMode = <TransportMode.Radiance: 0>) -> None
 |
 |      //! @}
 |
 |      2. __init__(self: mitsuba.BSDFContext, mode: mitsuba.TransportMode, type_mask: int, component: int) -> None
 |
...

And mitsuba's generate_stub_files.py just copies that over to the .pyi file, but should maybe remove or transform the default argument instead somehow?

And similar for:

>>> help(mitsuba.Mesh)
Help on class Mesh in module mitsuba.scalar_rgb:

class Mesh(Shape)
 |  Method resolution order:
 |      Mesh
 |      Shape
 |      mitsuba.Object
 |      pybind11_builtins.pybind11_object
 |      builtins.object
 |
 |  Methods defined here:
 |
 |  __init__(...)
 |      __init__(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. __init__(self: mitsuba.scalar_rgb.Mesh, props: mitsuba.scalar_rgb.Properties) -> None
 |
 |      2. __init__(self: mitsuba.scalar_rgb.Mesh, name: str, vertex_count: int, face_count: int, props: mitsuba.scalar_rgb.Properties = Properties[
 |        plugin_name = "",
 |        id = "",
 |        elements = {
 |        }
 |      ]
 |      , has_vertex_normals: bool = False, has_vertex_texcoords: bool = False) -> None
 |
...

generate_stub_files.py maybe doesn't support that multi-line definition yet, caused by Mesh constructor const Properties &props = Properties() default argument?

petsuter avatar Aug 02 '22 09:08 petsuter

Thanks for the investigation @petsuter. We'll probably have to wait until we're back from SIGGRAPH to work on a fix, but this is helpful!

merlinND avatar Aug 02 '22 11:08 merlinND

This is a know issue and will be fixed once we have ported Mitsuba 3 to nanobind. Let's keep this issue open until then.

Speierers avatar Aug 15 '22 11:08 Speierers

Is there a workaround for this? E.g. some mypy parameter that I'm not aware of?

maxfrei750 avatar Sep 14 '22 07:09 maxfrei750

This was fixed and will be updated in the next release (in the coming days).

Speierers avatar Sep 14 '22 07:09 Speierers

Cool! However, is this already in the master branch? I just freshly cloned and compiled it, but I still get:

/src/mitsuba3/build/python/mitsuba/__init__.pyi:870: error: invalid syntax  [syntax]
Found 1 error in 1 file (errors prevented further checking)

maxfrei750 avatar Sep 14 '22 07:09 maxfrei750

I had implemented a fix here: https://github.com/mitsuba-renderer/mitsuba3/commit/4302caa8bfd200a0edd6455ba64f92eab2be5824#diff-a53b672cfec91d013ba8e76344cf343778d84ef9d7bfaf5c7727026a595a0c71R224-R235 It was something I had ported from the stub generator for Dr.Jit, but apparently it doesn't cover all of these use cases in Mitsuba :sweat: . I'll try to fix it.

Either way, the stub generation is not perfect. I would recommend ignoring mitsuba and drjit in mypy or any other type-checking tool. The stubs are (in their current state) mostly only intended for auto-completion purposes. As @Speierers mentionned, nanobind would resolve some issues that would require a lot more time/code to fix on pybind11. I'm happy to review or help with any PR that would improve the current stub generation :smile: .

njroussel avatar Sep 14 '22 08:09 njroussel

Thanks for your effort!

I would recommend ignoring mitsuba and drjit in mypy

That was my original idea. However, mypys ignore_errors option won't do the trick, since it's no typing error, but a syntax error. The --no-site-packages option (see https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-site-packages) helps, but also affects other packages. I think there should be a way using the --exclude option, but I couldn't make it work so far. Maybe I'm just not fluent enough with regular expressions. :sweat_smile:

Maybe you have an idea for a workaround?

maxfrei750 avatar Sep 14 '22 08:09 maxfrei750

FYI this elaborate workaround worked to avoid the syntax errors for me:

from typing import TYPE_CHECKING
if TYPE_CHECKING:
    # mitsuba 3.0.1 .pyi file contains invalid syntax https://github.com/mitsuba-renderer/mitsuba3/issues/155
    import mitsuba_workaround as mitsuba
else:
    import mitsuba

together with this mypy config file:

[mypy-mitsuba]
ignore_missing_imports = True
 
[mypy-mitsuba_workaround]
ignore_missing_imports = True

petsuter avatar Sep 14 '22 16:09 petsuter

I've made a few improvement to the stub generation in this commit sequence: b7ef349e063b632c41c94243f5df4bdffe2a9ab4..ad72a5361889bcef1f19b702a28956c1549d26e3

These improvements fix all syntax issues. But, there are still some remaining type errors which will still cause mypy to fail on a simple import mitsuba test. I will close this issue as it directly relates to the initial syntax issues.

For anyone looking to use static type checking in a project which imports mitsuba. I can only recommend @petsuter's suggestion to circumvent any errors. Unfortunately, the type information (stub) files are generated manually and is a lesser priority of ours. The script we use might break over time, and there are definitely improvements that can still be made. As mentionned previously, we would welcome any help with maintaining this part of the repository!

njroussel avatar Sep 21 '22 12:09 njroussel