napari / cookiecutter-napari-plugin

Cookiecutter for napari plugins
BSD 3-Clause "New" or "Revised" License
69 stars 41 forks source link

Use copier instead of cookiecutter #165

Closed GenevieveBuckley closed 1 year ago

GenevieveBuckley commented 1 year ago

This PR swaps copier for cookiecutter. It is a fully working implementation that people can try out (as suggested by @jni, see context below)

https://github.com/napari/napari/issues/6122 by @jni

Consider using copier instead of cookiecutter #6122

Some folks from the scientific Python ecosystem seem to like it.

One thing I like is that it has built-in support for updating projects when the template is updated: https://copier.readthedocs.io/en/stable/updating

Apparently cruft does this for cookiecutter templates but that seems like a more complex tool chain since it's not built into cookiecutter.

I don't have a strong opinion here, just want to share something I came across that could be interesting to implement, since updating downstream projects is something we have needed a lot of.

GenevieveBuckley commented 1 year ago

Some first thoughts:

DragaDoncila commented 1 year ago

I pulled this down and played with it a bit. In general I do like it - I like how much more flexibility there is over the prompts. It looks like the copier.yml file also supports when clauses which should allow us to provide more branching options to the user e.g. we could have a barebones plugin vs. a fully templated plugin with more ease than in the cookiecutter.

I don't like that basically every file in the template ends with .jinja because it means VsCode doesn't automatically lint/syntax highlight it (you have to manually select python, yaml etc. but then it does work!), but it's not a dealbreaker for me.

I don't know if this is something we can configure on our end but right now you have to use --trust at the command line otherwise you get an error:

Template uses potentially unsafe features: jinja_extensions, tasks.
If you trust this template, consider adding the `--trust` option when running `copier copy/update`.

Also, I needed to manually install Jinja2-time and it would be great if we could avoid that for users - but we might just have to provide that in our instructions.

Finally, I think currently the install_precommit choice is ignored, because I chose no and it still installed pre-commit.

GenevieveBuckley commented 1 year ago

jinja2-time

Also, I needed to manually install Jinja2-time and it would be great if we could avoid that for users - but we might just have to provide that in our instructions.

I added a line about installing copier and jinja2-time to the README, and added both to the requirements.txt. I think your suggestion has already been implemented.

Also, we only use the jinja2-time plugin to produce whatever today's date is in the LICENSE files for repositories created with this template. If adding a specific date to the license isn't necessary, we could remove it altogether.

--trust

I don't know if this is something we can configure on our end but right now you have to use --trust at the command line otherwise you get an error

This is unavoidable if you want to use jinja extensions, tasks, or migrations, since all three copier features allow the execution of arbitrary (and thus potentially unsafe) code. We can't configure this in the copier.yml, the user is required to add either --trust (or --unsafe) to their command line call. Here is the section of the docs with more information: https://copier.readthedocs.io/en/stable/configuring/#unsafe

  1. Tasks: we currently use tasks to run the _tasks.py script (validates the manifest, checks module name is PEP8 compliant, no hyphens in plugin name, optionally ). Previously these were cookiecutter hooks named pre_gen_project.py and post_gen_project.py
  2. jinja extensions: we currently only use jinja2-time, as described above
  3. Migrations: we do not use migrations right now.

It's my understanding that the tasks are important to run, but if there's a way to rework that perhaps you can avoid the --trust command line argument.

GenevieveBuckley commented 1 year ago

Finally, I think currently the install_precommit choice is ignored, because I chose no and it still installed pre-commit.

Hmm, that's not great 😦

DragaDoncila commented 1 year ago

Discussed at the PR party with @GenevieveBuckley, @jni and @psobolewskiPhD. We want to move forward with copier but don't want to overwrite this repo, as a lot of people might still be using it. We'll eventually fully deprecate this, but in the meantime we should continue with the copier template over at napari/napari-plugin-template

GenevieveBuckley commented 1 year ago

Closing in favour of https://github.com/napari/napari-plugin-template/pull/1