Simplify and improve template #8

jcfr · 2019-05-19T00:42:37Z

This PR implements the following:

simpler maintenance of the template by making use of nwb-docutils to generate the documentation.
addition of the following post generation hooks:
- creation of extension spec executing script create_extension_spec.py
- initialization of git repository

Since a git repo is required to successfully build the documentation, this is done here.

[...] Creating file /tmp/simulation_output/spec/simulation_output.extensions.yaml with 1 data types Creating file /tmp/simulation_output/spec/simulation_output.namespace.yaml [...]

Previous example has been copied from the simulation_output extension and was causing warnings when attempting to build the documentation. Indeed, the "Compartments" type was undefined.

This commit ensures the "spec/*.yaml" files are copied to "src/pynwb/spec/*.yaml", doing so allows the file to be packages into the distributed wheel. It also removes the setting of "include_package_data=True" from the NEXTSTEP instructions, spec files are already packages by setting package_data.

oruebel

Rather than running the init_sphinx_extension_docs script I think we could just make the files it creates part of the cookiecutter and remove the init_sphinx_extension_docs.

This commit ensures the extension python package is named after the namespace. Doing so avoids to override file like "pynwb/__init__.py" provided by the upstream "pynwb" package.

jcfr · 2019-05-19T09:07:35Z

Rather than running the init_sphinx_extension_docs script I think we could just make the files it creates part of the cookiecutter and remove the init_sphinx_extension_docs.

Do you think the original reasons leading to the implementation of init_sphinx_extension_docs are not valid anymore ? (it allows a fair amount of customization)

EDIT: To follow up, since nwb-docutils should be moved to a nwb agnostic location (see hdmf-dev/hdmf#7), removing init_sphinx_extension_docs from it is more relevant.

Fix python package layout to avoid overriding pynwb installation

To avoid overriding the files from the upstream pynwb package, the ndx python extension package must have a dedicated name.

Indeed, since we can't have pynwb/__init__.py, this last commit updates the template so that we have the following in the source tree:

<root>/src/pynwb/ndx_my_extension/__init__.py
<root>/spec/*.yaml

windows failure

These are due to using a folder name {{ cookiecutter.namespace|replace("-", "_") }}, I will implement a fix later tomorrow.

jcfr · 2019-05-19T09:12:18Z

Once, this last failure is addressed ... the remaining feature to implement will be the use of an entrypoints allowing to automatically register the ndx python extension on installation and allows any of the following:

listing installed extensions using a pynwb functions (e.g pnwb.list_registered_namespaces())
automatic loading of registered namespace on import. Do we want that ? @oruebel
implement a function named pynwb.load_registered_namespaces()

…iable See https://cookiecutter.readthedocs.io/en/latest/advanced/templates_in_context.html

rly · 2019-05-23T01:20:26Z

Rather than running the init_sphinx_extension_docs script I think we could just make the files it creates part of the cookiecutter and remove the init_sphinx_extension_docs.

Do you think the original reasons leading to the implementation of init_sphinx_extension_docs are not valid anymore ? (it allows a fair amount of customization)

EDIT: To follow up, since nwb-docutils should be moved to a nwb agnostic location (see hdmf-dev/hdmf#7), removing init_sphinx_extension_docs from it is more relevant.

@oruebel Should we remove init_sphinx_extension_docs and make it part of the cookiecutter then? Sorry @jcfr I know you spent a lot of time on this...

oruebel · 2019-05-23T01:32:53Z

@oruebel @jcfr whatever makes most sense to you with regard to the init_sphinx_extensions_docs is fine with me. If it is useful or easier to keep using the script then that's fine with me too. I just thought it would make things simpler to just have it part of the cookiecutter.

jcfr · 2019-05-23T02:02:09Z

Should we remove init_sphinx_extension_docs and make it part of the cookiecutter then?

It could be added to the hooks module here: https://github.com/nwb-extensions/ndx-template/tree/master/hooks

But if we don't anticipate the user running the CLI locally to update their documentation, we could simply get ride of it.

To get ride of it suggest the following step:

generate an extension using cookie cutter
update the cookie cutter
re-generate and compare using tool like meld

Since I already have a local script to support that workflow, I could fairly quickly update ndx-template so that we eliminate init_sphinx_extensions_docs. Let me know

rly · 2019-05-23T02:13:04Z

OK. Yeah, I think we should replace init_sphinx_extensions_docs with cookiecutter template files. It seems a little easier to maintain the cookiecutter template than the init_sphinx_extensions_docs script. e.g. helpful IDE features such as syntax coloring and Markdown previews do not work on the strings of Python/Markdown code within init_sphinx_extensions_docs. It is also a little easier to see and debug what cookiecutter is doing than the python script.

While the init_sphinx_extensions_docs does allow for some extra customization at the command line, I don't think that customization will ever be used.

@jcfr It would be great if you could make this change. Let me know if you would like a hand.

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch 2 times, most recently from b19009a to 014182f Compare May 19, 2019 00:49

Simplify template maintenance generating docs using nwb-docutils

22152a0

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch from 014182f to 22152a0 Compare May 19, 2019 00:51

jcfr requested review from rly and oruebel May 19, 2019 00:52

jcfr mentioned this pull request May 19, 2019

Cross-reference docs to other extensions #1

Open

jcfr added 6 commits May 19, 2019 02:04

hooks/post_gen_project: Initialize git repo

e887d33

Since a git repo is required to successfully build the documentation, this is done here.

src/spec/export_spec.py: Improved displayed messages

2d05ad7

[...] Creating file /tmp/simulation_output/spec/simulation_output.extensions.yaml with 1 data types Creating file /tmp/simulation_output/spec/simulation_output.namespace.yaml [...]

hooks/post_gen_project: Generate extension spec files

f30bd04

create_extension_spec: Update example to match pynwb tutorial

aa63d7e

Previous example has been copied from the simulation_output extension and was causing warnings when attempting to build the documentation. Indeed, the "Compartments" type was undefined.

tests: Check that generated files exists and are not empty

77c9e7d

ci: Fix template evaluation configuring git user.email and user.name

9e14acd

jcfr changed the title ~~Simplify template maintenance generating docs using nwb-docutils~~ Simplify and improve template May 19, 2019

ci: Check that building documentation succeeds

384c488

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch from 3e8fa2b to 384c488 Compare May 19, 2019 06:51

jcfr added 2 commits May 19, 2019 03:59

Fix generation of extension sdist adding MANIFEST.in

57993db

Fix generate of extension wheel ensuring "pynwb" package is found

10d60b0

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch from 68592d6 to e2bd453 Compare May 19, 2019 08:41

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch from e2bd453 to 188b62d Compare May 19, 2019 08:45

oruebel approved these changes May 19, 2019

View reviewed changes

jcfr added 2 commits May 19, 2019 04:54

Fix generation of sdist ensuring all docs files are included

09e01e5

Fix python package layout to avoid overriding pynwb installation

526638e

This commit ensures the extension python package is named after the namespace. Doing so avoids to override file like "pynwb/__init__.py" provided by the upstream "pynwb" package.

Fix use of template on windows introducing "py_pkg_name" template var…

1c490cf

…iable See https://cookiecutter.readthedocs.io/en/latest/advanced/templates_in_context.html

jcfr force-pushed the nwb-docutil-as-postgen-hook-to-generate-docs branch from 0159762 to 1c490cf Compare May 19, 2019 09:28

jcfr merged commit 7c99eac into nwb-extensions:master May 19, 2019

jcfr deleted the nwb-docutil-as-postgen-hook-to-generate-docs branch May 19, 2019 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify and improve template #8

Simplify and improve template #8

jcfr commented May 19, 2019 •

edited

Loading

oruebel left a comment

jcfr commented May 19, 2019 •

edited

Loading

jcfr commented May 19, 2019 •

edited

Loading

rly commented May 23, 2019

oruebel commented May 23, 2019

jcfr commented May 23, 2019

rly commented May 23, 2019

Simplify and improve template #8

Simplify and improve template #8

Conversation

jcfr commented May 19, 2019 • edited Loading

oruebel left a comment

Choose a reason for hiding this comment

jcfr commented May 19, 2019 • edited Loading

jcfr commented May 19, 2019 • edited Loading

rly commented May 23, 2019

oruebel commented May 23, 2019

jcfr commented May 23, 2019

rly commented May 23, 2019

jcfr commented May 19, 2019 •

edited

Loading

jcfr commented May 19, 2019 •

edited

Loading

jcfr commented May 19, 2019 •

edited

Loading