funkybob / gilbert

A simple static site generator.
https://gilbert.readthedocs.io/en/latest/
MIT License
10 stars 4 forks source link

[MRG] Attempt to load default plugins first #29

Closed shuttle1987 closed 5 years ago

shuttle1987 commented 5 years ago

This is a rather quick fix approach to the default plugins being sometimes loaded after the user installed ones. Will close #28

shuttle1987 commented 5 years ago

I don't like that the two loops replicate a lot of work.

I don't like this either.

I'm just not quite sure how to use the path here due to the namespace packages being involved. This is what it looks like in my site-packages after I install gilbert and the markdown-frontmatter loader:

digital_site/venv/lib/python3.7/site-packages/gilbert (render-markdown)$  tree .
.
├── cli.py
├── collection.py
├── content.py
├── exceptions.py
├── __init__.py
├── __main__.py
├── plugins
│   ├── collection.py
│   ├── frontmatter_markdown.py
│   ├── markdown.py
│   ├── __pycache__
│   │   ├── collection.cpython-37.pyc
│   │   ├── frontmatter_markdown.cpython-37.pyc
│   │   ├── markdown.cpython-37.pyc
│   │   ├── scss.cpython-37.pyc
│   │   └── yaml.cpython-37.pyc
│   ├── scss.py
│   └── yaml.py
├── __pycache__
│   ├── cli.cpython-37.pyc
│   ├── collection.cpython-37.pyc
│   ├── content.cpython-37.pyc
│   ├── exceptions.cpython-37.pyc
│   ├── __init__.cpython-37.pyc
│   ├── __main__.cpython-37.pyc
│   ├── query.cpython-37.pyc
│   ├── schema.cpython-37.pyc
│   ├── site.cpython-37.pyc
│   └── utils.cpython-37.pyc
├── query.py
├── schema.py
├── site.py
└── utils.py

3 directories, 30 files

Is there some other way to look at the path that I'm not aware of here?

funkybob commented 5 years ago

Python helps us a little here by letting us access the gilbert.plugins.__path__ attribute which [despite the name] is a list of paths contributing to this namespace.

However, we can also use __file__ to find the current module's directory, and then list the packages under there - sort of side-stepping our use of gilbert.plugins.__path__.

So my thinking is we build a list of modules that we want to import; first we iterate our "built in" list of packages, add them to the list, then we iterate the whole list [much like we do now], but check for duplicates. Finally, we iterate the list and import them.

This splits up the existing loop into multiple stages, but each stage does not replicate the work of the others.

shuttle1987 commented 5 years ago

I only seem to get one path though (I'm not installing in editable mode), putting this into site.py load_plugins:

        import gilbert
        print("***")
        print(f"__file__: {__file__}")
        print(f"Paths in {gilbert.plugins.__path__}:")
        for p in gilbert.plugins.__path__:
            print(p)
        print("***")

then gives the following output on load:

$ gilbert render
***
__file__: /home/janis/digital_site/venv/lib/python3.7/site-packages/gilbert/site.py
Paths in _NamespacePath(['/home/janis/digital_site/venv/lib/python3.7/site-packages/gilbert/plugins']):
/home/janis/digital_site/venv/lib/python3.7/site-packages/gilbert/plugins
***
Searching /home/janis/digital_site/venv/lib/python3.7/site-packages/gilbert/plugins for default plugins...
Loaded plugin: collection
Loaded plugin: scss
Loaded plugin: markdown
Loaded plugin: yaml
Searching /home/janis/digital_site/venv/lib/python3.7/site-packages/gilbert/plugins for plugins...
Loaded the Gilbert markdown with frontmatter loader
WARNING: Overriding loader for md
Loaded plugin: frontmatter_markdown
Loaded local plugins.

If your suggestion was to build the import paths for the default plugins first I see how this could work. But if there's some way to find the defaults from the path I guess I'm just not sure how to deal with only having the one path here (the site-pacakges one). (if the default plugins lived in their own path I see how this could be very easy to put together)

shuttle1987 commented 5 years ago

Specifically:

So my thinking is we build a list of modules that we want to import; first we iterate our "built in" list of packages, add them to the list, then we iterate the whole list [much like we do now], but check for duplicates. Finally, we iterate the list and import them.

I really like this approach, I'm just not sure how to do it with the current package structure without hard-coding the modules that are defaults. If there's a way to find the default plugins dynamically from the path I'll add that into the PR.

funkybob commented 5 years ago

I only seem to get one path though (I'm not installing in editable mode), putting this into site.py load_plugins:

Does the setup.py for your plugin package use find_namespace ?

In my setup.cfg I use:

[options]
packages = find_namespace:

I thought this used to be in the docs, but can't find it.

shuttle1987 commented 5 years ago

It doesn't use this at the moment, I can edit it if this helps. At the moment I just have a simple setup.py: https://github.com/shuttle1987/gilbert-frontmatter-markdown/blob/19c9cb30cc6f1bf1b382b34d25e1af405788a56e/setup.py

funkybob commented 5 years ago

OK, i added some notes to the docs about using find_namespace_packages, but perhaps also we should link to https://setuptools.readthedocs.io/en/latest/setuptools.html#namespace-packages

shuttle1987 commented 5 years ago

Thanks for adding those docs, initially I just took the approach of installing into the same directory in site-packages as being sufficient but since these changes I see why that's no good. If I can find the time I'd like to write a blog post about what happened here and how the namespace packages work, I'll also fix my plugin so it installs in a different path.

shuttle1987 commented 5 years ago

I just can't for the life of me figure out how to get my plugin packages to be installed into different paths (and hence get the iteration out of NamespacePath like we want to handle the package loading behavior that detects the builtins)

funkybob commented 5 years ago

Are you working from the latest master? I made a small adjustment after reading the setuptools docs more myself.

funkybob commented 5 years ago

Some further investigation has shown: a) I was wrong about adding the init b) what we're seeing may be a result of "that's just how pip works" ( a simple test showed uninstalling the package removed only the expected files, which is good! )

shuttle1987 commented 5 years ago

If I understand correctly PR #31 makes this PR obsolete, feel free to reopen if not.