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

fix: catch recursion in Alias.resolved #115

Closed
wants to merge 2 commits into from

Conversation

tlambert03
Copy link
Contributor

@tlambert03 tlambert03 commented Nov 19, 2022

hey @pawamoy

I'm sure this is probably your least favorite bug at this point 😂 ... but I ran into an Alias resolution recursion error, similar to #83

Full traceback:
Traceback (most recent call last):
  File "/Users/talley/mambaforge/envs/mmcore/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/__main__.py", line 234, in serve_command
    serve.serve(dev_addr=dev_addr, livereload=livereload, watch=watch, **kwargs)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/commands/serve.py", line 83, in serve
    builder(config)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/commands/serve.py", line 76, in builder
    build(config, live_server=live_server, dirty=dirty)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/commands/build.py", line 308, in build
    _populate_page(file.page, config, files, dirty)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/commands/build.py", line 181, in _populate_page
    page.render(config, files)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocs/structure/pages.py", line 270, in render
    self.content = md.convert(self.markdown)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/markdown/core.py", line 264, in convert
    root = self.parser.parseDocument(self.lines).getroot()
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/markdown/blockparser.py", line 90, in parseDocument
    self.parseChunk(self.root, '\n'.join(lines))
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/markdown/blockparser.py", line 105, in parseChunk
    self.parseBlocks(parent, text.split('\n\n'))
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/markdown/blockparser.py", line 123, in parseBlocks
    if processor.run(parent, blocks) is not False:
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocstrings/extension.py", line 121, in run
    html, handler, data = self._process_block(identifier, block, heading_level)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocstrings/extension.py", line 195, in _process_block
    data: CollectorItem = handler.collect(identifier, options)
  File "/Users/talley/mambaforge/envs/mmcore/lib/python3.10/site-packages/mkdocstrings_handlers/python/handler.py", line 199, in collect
    unresolved, iterations = loader.resolve_aliases(implicit=False, external=False)
  File "/Users/talley/dev/forks/griffe/src/griffe/loader.py", line 217, in resolve_aliases
    next_resolved, next_unresolved = self.resolve_module_aliases(module, implicit, external)
  File "/Users/talley/dev/forks/griffe/src/griffe/loader.py", line 357, in resolve_module_aliases
    sub_resolved, sub_unresolved = self.resolve_module_aliases(
  File "/Users/talley/dev/forks/griffe/src/griffe/loader.py", line 338, in resolve_module_aliases
    member.resolve_target()  # type: ignore[union-attr]
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 967, in resolve_target
    self._target.aliases[self.path] = self  # type: ignore[union-attr]  # we just set the target
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 827, in __getattr__
    attr = getattr(self.target, name)
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 935, in target
    if not self.resolved:
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 976, in resolved
    return self._target is not None and (not self._target.is_alias or self._target.resolved)  # type: ignore[union-attr]
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 976, in resolved
    return self._target is not None and (not self._target.is_alias or self._target.resolved)  # type: ignore[union-attr]
  File "/Users/talley/dev/forks/griffe/src/griffe/dataclasses.py", line 976, in resolved
    return self._target is not None and (not self._target.is_alias or self._target.resolved)  # type: ignore[union-attr]
  [Previous line repeated 971 more times]
RecursionError: maximum recursion depth exceeded

In this case, it does appear to be a legitimate Cyclic Alias... that is self._target._target is self.

For what it's worth, the class I was trying to document with mkdocstrings is from pymmcore (pip install pymmcore)

::: pymmcore.CMMCore

... and that class is a SWIG wrapper with a .pyi type stub providing the signatures and docstring. And the cyclic aliases look like:

<Alias('CMMCore', 'pymmcore.pymmcore_swig.CMMCore')>
<Alias('CMMCore', 'pymmcore._pymmcore_swig.CMMCore')>
<Alias('CMMCore', 'pymmcore.pymmcore_swig.CMMCore')>
<Alias('CMMCore', 'pymmcore._pymmcore_swig.CMMCore')>
...

I opened this as a PR rather than an issue, since this does fix the docs building (the page looks exactly how I'd expect it to)... but I'm not sure what the actual best fix is. Feel free to close or suggest a better approach here for me to implement

@tlambert03 tlambert03 changed the title catch recursion catch recursion in Alias.resolved Nov 19, 2022
@pawamoy
Copy link
Member

pawamoy commented Nov 19, 2022

least favorite bug

Not agaaaaaaaaaaaaaaaaaaain 😂
Yeah I hate this error, but that's entirely my fault 👐
I'm working on fixing the design (implicitness triggering alias errors all the time), but I struggle to find something explicit
that would also not require wrapping every attribute access in a try/except or contextlib.suppress block 🤔

For now I'll continue to apply bandages on this poor project 😢

Thanks for the report and PR!

Catching recursion errors is like the absolute last resort haha.

I read SWIG wrapper + type stubs, I understand compiled module?
I recall having dealt with a similar case in Numpy. Here's the special case:

or parent_module_name == "numpy.core._multiarray_umath"
and child_module_name == "numpy.core.multiarray"

Maybe a similar situation here.

@tlambert03
Copy link
Contributor Author

Catching recursion errors is like the absolute last resort haha.

yep. I changed this PR to an attribute check, and that also fixes mkdocs build for me. But if you'd prefer to close it and look into a different fix, I won't be offended in the least :) and can open a bug issue as a reminder.

I read SWIG wrapper + type stubs, I understand compiled module?

yep, compiled module.

Maybe a similar situation here.

yeah I think so (pymmcore.pymmcore_swig.CMMCore -> pymmcore._pymmcore_swig.CMMCore -> pymmcore.pymmcore_swig.CMMCore)

@tlambert03 tlambert03 changed the title catch recursion in Alias.resolved fix: catch recursion in Alias.resolved Nov 19, 2022
pawamoy added a commit that referenced this pull request Nov 30, 2022
Also add support for the `pymmcore` project.

PR #115: #115
@pawamoy
Copy link
Member

pawamoy commented Nov 30, 2022

Closing in favor of 9a2a711 🙂

@pawamoy pawamoy closed this Nov 30, 2022
@tlambert03
Copy link
Contributor Author

wow 😮 ... special casing it huh? don't think it's gonna happen again?

@pawamoy
Copy link
Member

pawamoy commented Nov 30, 2022

It will probably happen again, but I want to improve the alias resolution mechanism, and maybe provide a way for registering cycles in this new "cyclic relationships" map, so that project suffering from it can fix it themselves.

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 this pull request may close these issues.

2 participants