Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate NMODL docs and improve their discoverability #2922

Merged
merged 21 commits into from
Jun 24, 2024

Conversation

kbvw
Copy link
Contributor

@kbvw kbvw commented Jun 18, 2024

Duplicate versions of the NMODL documentation pages currently exist under Python and HOC:

hoc/modelspec/programmatic/mechanisms/nmodl.html
hoc/modelspec/programmatic/mechanisms/nmodl2.html
python/modelspec/programmatic/mechanisms/nmodl.html
python/modelspec/programmatic/mechanisms/nmodl2.html

Very little content in these pages is specific to either HOC or Python, and the pages have diverged, with recent additions only added to the Python versions, and some earlier corrections only added to the HOC versions.

In addition, these pages are nearly impossible to find by clicking through the documentation, even though they serve as a main source of documentation for the NMODLanguage.

This PR merges the Python/HOC versions of these pages together and moves them to the following location:

nmodl/language/nmodl.html
nmodl/language/nmodl_neuron_extension.html

The (nearly empty) page previously at this location is renamed to nmodl/tutorials.html.

@kbvw
Copy link
Contributor Author

kbvw commented Jun 18, 2024

A few more possible improvements pending discussion:

  • The POINTER section exists both in nmodl.html and nmodl_neuron_extension.html (previously nmodl2.html). Presumable this section can be removed in the former page, as it concerns the NEURON block?
  • For the few sections that have Python/HOC-specific content (notably the above-mentioned POINTER section), the current solution states the examples after the other. This looks a bit messy and could be improved with language-specific tabs, if it is okay to add the sphinx-inline-tabs dependency. (Example: https://packaging.python.org/en/latest/guides/writing-pyproject-toml/.)
  • The tutorials page (previously the only page under the NMODLanguage tab) seems to be a messy draft consisting mostly of placeholders. I am hesitant about removing a page outright, but presumably it could be removed, and perhaps reintroduced in a better state later.

Many more things can be cleaned up about these pages, but discoverability and preventing the duplicates from diverging more would seem to be the priority.

Copy link

✔️ a6df245 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

✔️ fefd2df -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@JCGoran
Copy link
Contributor

JCGoran commented Jun 20, 2024

  • The POINTER section exists both in nmodl.html and nmodl_neuron_extension.html (previously nmodl2.html). Presumable this section can be removed in the former page, as it concerns the NEURON block?

Briefly skimming the source, I'd say yes, POINTER should be part of the NEURON block (and nrnivmodl errors out if you try putting it anywhere else), but just to be sure I'll let @ramcdougal and @nrnhines chime in.

  • For the few sections that have Python/HOC-specific content (notably the above-mentioned POINTER section), the current solution states the examples after the other. This looks a bit messy and could be improved with language-specific tabs, if it is okay to add the sphinx-inline-tabs dependency. (Example: https://packaging.python.org/en/latest/guides/writing-pyproject-toml/.)

If it'd improve readability, doesn't require too many changes, and doesn't have any conflicts with existing packages, I'm all for it!

  • The tutorials page (previously the only page under the NMODLanguage tab) seems to be a messy draft consisting mostly of placeholders. I am hesitant about removing a page outright, but presumably it could be removed, and perhaps reintroduced in a better state later.

I would be in favor of keeping it, but updating the links (see this page, links at the top need to be changed), and maybe putting a giant warning sign "UNDER CONSTRUCTION/DEVELOPMENT" for now, we can remove/modify it in another PR.

@kbvw
Copy link
Contributor Author

kbvw commented Jun 20, 2024

Thanks for the comments Goran!

For the turorials page, people in the meeting yesterday seemed to be in favour of removing it for now and adding it back later if there's time to work it out.

I'm now implementing the tabs so we can see how that looks, will push it later today.

@nrnhines
Copy link
Member

  • The POINTER section exists both in nmodl.html and nmodl_neuron_extension.html (previously nmodl2.html). Presumable this section can be removed in the former page, as it concerns the NEURON block?

Briefly skimming the source, I'd say yes, POINTER should be part of the NEURON block (and nrnivmodl errors out if you try putting it anywhere else), but just to be sure I'll let @ramcdougal and @nrnhines chime in.

I agree.

  • For the few sections that have Python/HOC-specific content (notably the above-mentioned POINTER section), the current solution states the examples after the other. This looks a bit messy and could be improved with language-specific tabs, if it is okay to add the sphinx-inline-tabs dependency. (Example: https://packaging.python.org/en/latest/guides/writing-pyproject-toml/.)

If it'd improve readability, doesn't require too many changes, and doesn't have any conflicts with existing packages, I'm all for it!

I agree

  • The tutorials page (previously the only page under the NMODLanguage tab) seems to be a messy draft consisting mostly of placeholders. I am hesitant about removing a page outright, but presumably it could be removed, and perhaps reintroduced in a better state later.

I would be in favor of keeping it, but updating the links (see this page, links at the top need to be changed), and maybe putting a giant warning sign "UNDER CONSTRUCTION/DEVELOPMENT" for now, we can remove/modify it in another PR.

I lean toward (temporary?) removal. The negative impression at the moment is pretty strong. Is it much help to a future documentation author?

@JCGoran
Copy link
Contributor

JCGoran commented Jun 20, 2024

Is it much help to a future documentation author?

The only thing that could be potentially helpful are the example mod files, which are sorely missing from the main docs (though admittedly the examples are also very sparsely documented), and having them in the docs somewhere as a future reference could be useful (rather than having to write them from scratch or digging them out from git history). Again, I've no strong opinion on this.

Copy link
Member

@nrnhines nrnhines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement. Forms a good basis for bringing some details up to date.

Copy link

✔️ 7706c51 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Copy link

sonarcloud bot commented Jun 21, 2024

Copy link

✔️ 3d60327 -> Azure artifacts URL

@kbvw
Copy link
Contributor Author

kbvw commented Jun 21, 2024

Thanks Goran and Michael for the comments!

  • The duplicate POINTER/Pointer Communication sections are now merged and only the latter is included, under nmodl_neuron_extension.html.
  • The same Pointer Communication section (and a few others like PROCEDURE in nmodl.html) now have synchronized mini tabs for Python/HOC syntax

The only thing that could be potentially helpful are the example mod files, which are sorely missing from the main docs (though admittedly the examples are also very sparsely documented), and having them in the docs somewhere as a future reference could be useful (rather than having to write them from scratch or digging them out from git history). Again, I've no strong opinion on this.

Regarding the tutorial page, I removed the small .rst file that simply included the notebook, and it's entry from the index. And then I kept the .ipynb notebook with the contents. (Admittedly also because, being the only notebook in the repo, removing it seemed to break some notebook-related CI.) So now it's invisible, but still around if someone wants to flesh it out and include it again.

@pramodk pramodk merged commit 6165884 into master Jun 24, 2024
35 of 36 checks passed
@pramodk pramodk deleted the consolidate_nmodl_docs branch June 24, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants