nwb-extensions / ndx-template

A template for creating Neurodata Extensions for the NWB neurophysiology data standard
https://nwb.org/
Other
5 stars 8 forks source link

Simplify and improve template #8

Closed jcfr closed 5 years ago

jcfr commented 5 years ago

This PR implements the following:

jcfr commented 5 years ago

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 https://github.com/hdmf-dev/hdmf/issues/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 commented 5 years ago

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:

rly commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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:

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 commented 5 years ago

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.