Add note about not using editable installs for docs #209

melissawm · 2023-07-24T21:06:29Z

Description

Adds note about building documentation with editable builds of napari.

Type of change

Fixes or improves existing content
Adds new content page(s)
Fixes or improves workflow, documentation build or deployment

References

Related to #207 (comment)

Final checklist:

My PR is the minimum possible work for the desired functionality
I have commented my code, particularly in hard-to-understand areas
I have added alt text to new images included in this PR

jni · 2023-07-24T23:39:33Z

docs/developers/documentation/index.md

+([sphinx-gallery](https://sphinx-gallery.github.io/stable/index.html)), you may
+need to install napari from source without using an editable install if your
+goal is to build the documentation locally. In that case, you should not use the
+`-e` argument of `pip install`.


Thanks for this @melissawm! I think without the context of #207 and the comments, this is a bit vague. I suggest adding language like: "If you see an error like the following: AttributeError: module 'napari.Viewer' has no attribute 'close_all', then make sure you install napari without an editable install. That is, don't use-e to install a local version of napari. For more details, see the discussion at napari/docs#207."

Yeah, I am debating not expanding too much to not be overwhelming for someone who only wants to build docs and giving full context. Maybe this isn't even necessary if we find out the source of the original error? I'll mark this PR as draft and explore a bit.

Czaki · 2023-07-25T11:50:38Z

Could you point out your setup? I was able to build docs using editable installation without a problem.

melissawm · 2023-07-25T12:52:57Z

@Czaki the context is:

On an intel mac, create a fresh conda environment (python 3.10 or 3.11, doesn't matter)
clone napari from source and do an editable install with python -m pip install -e ".[dev,pyqt]"
clone napari/docs and make docs

This generates errors that do not appear if the install from source is not set to editable.

Traceback (most recent call last):
  File "/Users/melissa/mambaforge/envs/napari-dev/lib/python3.11/site-packages/sphinx_gallery/scrapers.py", line 340, in save_figures
    rst = scraper(block, block_vars, gallery_conf)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/melissa/projects/napari-docs/docs/conf.py", line 236, in napari_scraper
    napari.Viewer.close_all()
    ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'napari.Viewer' has no attribute 'close_all'

This may be a sphinx-gallery issue, but since we could work around it by not using an editable install (and this may be easier than debugging sphinx-gallery) I am proposing this here. But now I'm reconsidering, maybe there is a fix.

Czaki · 2023-07-25T12:59:27Z

If you could modify the code:

try:
    napari.Viewer.close_all()
except AttributteError:
    print(napari.__file__, dir(napari.Viewer))
    raise

and paste the result

melissawm · 2023-07-25T13:43:19Z

@Czaki here's the output:

WARNING: /Users/melissa/projects/napari/examples/custom_mouse_functions.py failed to execute correctly: Traceback (most recent call last):                                                                     
  File "/Users/melissa/projects/napari/examples/custom_mouse_functions.py", line 18, in <module>
    viewer = napari.Viewer()
TypeError: 'module' object is not callable

/Users/melissa/projects/napari/napari/__init__.py ['Optional', 'TYPE_CHECKING', 'Viewer', 'ViewerModel', 'WeakSet', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', '_magicgui', 'current_viewer', 'mgui', 'sys', 'typing']

Extension error (sphinx_gallery.gen_gallery):
Handler <function generate_gallery_rst at 0x10eed7ac0> for event 'builder-inited' threw an exception (exception: module 'napari.Viewer' has no attribute 'close_all')
make: *** [docs-build] Error 2
(napari-dev) ➜  napari-docs git:(main) ✗

Czaki · 2023-07-25T14:07:53Z

If a Viewer ismodule could you add points of it's path? (print(napari.Viewer.file))

melissawm · 2023-07-25T14:34:25Z

Using jupyter-cache at: /Users/melissa/projects/napari-docs/docs/.jupyter_cache
generating gallery...
WARNING: /Users/melissa/projects/napari/examples/3D_paths.py failed to execute correctly: Traceback (most recent call last):                                                                                   
  File "/Users/melissa/mambaforge/envs/napari-dev/lib/python3.10/site-packages/sphinx_gallery/scrapers.py", line 340, in save_figures
    rst = scraper(block, block_vars, gallery_conf)
  File "/Users/melissa/projects/napari-docs/docs/conf.py", line 238, in napari_scraper
    print(napari.Viewer.__file__)
AttributeError: type object 'Viewer' has no attribute '__file__'

Czaki · 2023-07-25T15:40:54Z

wait. It if is module it should have __file__ Maybe it is in some example

try:
    napari.Viewer.close_all()
except AttributteError:
    print(napari.__file__, getattr(napari.Viewer, "__file__", "Nope"))
    raise

melissawm · 2023-07-25T16:08:05Z

With

def napari_scraper(block, block_vars, gallery_conf):
    """Basic napari window scraper.

    Looks for any QtMainWindow instances and takes a screenshot of them.

    `app.processEvents()` allows Qt events to propagateo and prevents hanging.
    """
    imgpath_iter = block_vars['image_path_iterator']

    if app := napari.qt.get_app():
        app.processEvents()
    else:
        return ""

    img_paths = []
    for win, img_path in zip(
        reversed(napari._qt.qt_main_window._QtMainWindow._instances),
        imgpath_iter,
    ):
        img_paths.append(img_path)
        win._window.screenshot(img_path, canvas_only=False)

    try:
        napari.Viewer.close_all()
    except AttributeError:
        print(napari.__file__, getattr(napari.Viewer, "__file__", "Nope"))
        raise

    app.processEvents()

    return scrapers.figure_rst(img_paths, gallery_conf['src_dir'])

I get

WARNING: /Users/melissa/projects/napari/examples/custom_mouse_functions.py failed to execute correctly: Traceback (most recent call last):                                                                     
  File "/Users/melissa/projects/napari/examples/custom_mouse_functions.py", line 18, in <module>
    viewer = napari.Viewer()
TypeError: 'module' object is not callable

/Users/melissa/projects/napari/napari/__init__.py /Users/melissa/projects/napari/napari/Viewer.py

Extension error (sphinx_gallery.gen_gallery):
Handler <function generate_gallery_rst at 0x107247ac0> for event 'builder-inited' threw an exception (exception: module 'napari.Viewer' has no attribute 'close_all')
make: *** [docs-build] Error 2
(napari-dev) ➜  napari-docs git:(main) ✗

aganders3 · 2023-07-27T15:20:58Z

Sorry I just noticed this issue. I've been digging into this problem. It's caused by a couple of things working together against us:

sphinx-gallery does some module importing when looking for backreferences, and this can leave new imports in sys.modules. I am discussing possible improvements here with @lucyleeow.
setuptools allows case-insensitive imports on certain systems (macOS) with editable installs. I have opened an issue there to see if it's something they are open to changing ([BUG] certain editable installs allow case-insensitive imports pypa/setuptools#3994).

Together these create our problem in the docs build. Here are the steps to failure as I understand them:

custom_key_bindings.py (maybe others) access napari.Viewer.bind_key()
sphinx-gallery searches for backreferences and in the process tries from napari.Viewer import bind_key
The above import fails, but the damage is done - napari.Viewer is now in sys.modules and points to napari/viewer.py, this part of the import succeeds due to the setuptools issue with case-sensitivity
The next time napari.Viewer is accessed, it bypasses the getattr lazy-loading in napari/__init__.py and instead gives back the module napari.viewer (with the wrong capitalized name). This can happen in the scraper where it accesses napari.Viewer.close_all(), or in the next example that uses napari.Viewer().

Current workarounds (if there are no changes to setuptools or sphinx-gallery) are to install napari as non-editable, or to use pip install -e ./napari --config-settings editable_mode=strict.

aganders3 · 2023-07-28T14:30:50Z

After a bit more investigation there's slightly more to it.

from napari.Viewer import bind_key adds napari.Viewer to sys.modules, which can be cleaned up by sys.modules.pop("napari.Viewer", None)

...but this also setssys.modules["napari"].__dict__["Viewer"] to napari.viewer (the module). This is a bit hairier to clean up completely, but it might be good to at least make an effort.

lucyleeow · 2023-07-29T11:00:44Z

This may not be relevant, but if we miss a reference to the napari.viewer module during clean up, what happens on the next import? If we import it again, I don't think the 2 module objects would be the same:

Beware though, as if you keep a reference to the module object, invalidate its cache entry in sys.modules, and then re-import the named module, the two module objects will not be the same.

ref: https://docs.python.org/3/reference/import.html#the-module-cache

But the question is, would all references to this module in sys.modules be updated on the next import or only the ones we cleaned up? If it is the latter, we may have the situation where module references that should point to the same module object, point to differing module objects, though I am not sure if this matters.

aganders3 · 2023-07-31T14:17:32Z

But the question is, would all references to this module in sys.modules be updated on the next import or only the ones we cleaned up? If it is the latter, we may have the situation where module references that should point to the same module object, point to differing module objects, though I am not sure if this matters.

Yes I think you're right this could happen. As with a lot of this stuff it's probably unlikely to cause a problem, but I'm sure it could (for example if using module-scope variables).

I'm increasingly convinced the only truly safe way to test imports like this would be to do it in a subprocess. I tested this and it's predictably rather slow. Still, there's probably some reasonable best-effort that can be made.

In any case we may be sufficiently off-topic for this PR 😅. I recommend keeping some simple wording here as proposed and we can revisit removing the language if/when fixes and workarounds are merged upstream.

melissawm · 2023-07-31T16:29:39Z

Ok - I have to say i don't really follow the full discussion so I'll mark this as ready for review and if we can do better on the sphinx-gallery itself we can revisit this note later. Thanks everyone!

aganders3 · 2023-07-31T17:17:32Z

I will say I agree with @jni's wording suggestions. I don't want to throw sphinx-gallery under the bus - it's at least not a problem singularly caused by that package. Plus, this is an intermittent problem depending on platform so I like the "If you see errors such as..." phrasing.

lucyleeow · 2023-08-01T03:16:04Z

I've created a new issue as a better place to have this discussion! #214

aganders3 · 2023-08-23T16:48:24Z

We might be able to close this now as a fix for these case-insensitive imports has been released in setuptools 68.1.0.

lucyleeow · 2023-08-23T17:04:45Z

Thanks @aganders3 !

Add note about not using editable installs for docs

c376e6d

melissawm mentioned this pull request Jul 24, 2023

Use napari_scraper instead of qtgallery #207

Merged

3 tasks

github-actions bot assigned melissawm Jul 24, 2023

github-actions bot added the documentation Improvements or additions to documentation label Jul 24, 2023

jni reviewed Jul 24, 2023

View reviewed changes

melissawm marked this pull request as draft July 25, 2023 12:50

Merge branch 'main' into docs-gallery

b0bd633

melissawm marked this pull request as ready for review July 31, 2023 16:29

lucyleeow mentioned this pull request Aug 1, 2023

Examples gallery fails on case insensitive/case preserving systems when napari installed as editable #214

Closed

lucyleeow closed this Aug 23, 2023

psobolewskiPhD added this to the 0.4.19 milestone Oct 24, 2023

melissawm deleted the docs-gallery branch January 9, 2024 14:10

Add note about not using editable installs for docs #209

Add note about not using editable installs for docs #209

Uh oh!

Conversation

melissawm commented Jul 24, 2023

Description

Type of change

References

Final checklist:

Uh oh!

jni Jul 24, 2023

Choose a reason for hiding this comment

Uh oh!

melissawm Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Czaki commented Jul 25, 2023

Uh oh!

melissawm commented Jul 25, 2023

Uh oh!

Czaki commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melissawm commented Jul 25, 2023

Uh oh!

Czaki commented Jul 25, 2023

Uh oh!

melissawm commented Jul 25, 2023

Uh oh!

Czaki commented Jul 25, 2023

Uh oh!

melissawm commented Jul 25, 2023

Uh oh!

aganders3 commented Jul 27, 2023

Uh oh!

aganders3 commented Jul 28, 2023

Uh oh!

lucyleeow commented Jul 29, 2023

Uh oh!

aganders3 commented Jul 31, 2023

Uh oh!

melissawm commented Jul 31, 2023

Uh oh!

aganders3 commented Jul 31, 2023

Uh oh!

lucyleeow commented Aug 1, 2023

Uh oh!

aganders3 commented Aug 23, 2023

Uh oh!

lucyleeow commented Aug 23, 2023

Uh oh!

Uh oh!

Czaki commented Jul 25, 2023 •

edited

Loading