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

Add introductory text for models in documentation #2416

Merged
merged 28 commits into from
Dec 6, 2022

Conversation

jessica-mitchell
Copy link
Contributor

@jessica-mitchell jessica-mitchell commented Jun 21, 2022

This PR adds introductory text to explain a bit about models in NEST.

@clinssen @pnbabu I added a section on NESTML that is copied from your docs, please comment if you have some other text you would like better.

There is a table of terms used in model names along with commonly used terms in literature. It's incomplete and in need of a model expert to help fill it out.

https://nest-test.readthedocs.io/en/add-intro-models/models/models-main.html

@jessica-mitchell jessica-mitchell added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jun 21, 2022
Copy link
Contributor

@clinssen clinssen left a comment

Choose a reason for hiding this comment

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

Just some initial ideas!

doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
@jessica-mitchell
Copy link
Contributor Author

@pnbabu @clinssen Thanks for your comments, I applied your suggestions, and I created a page on the installation for installing NEST with NESTML - it doesnt got into much detail. I think most of the information should be kept in your docs - the only thing that might be necessary imo, is if there is any additional setup to get NESTML to work with NEST aside from having them both installed. Also note that in #2289 I added a link to the global toc for NESTML.

@terhorstd
Copy link
Contributor

@jessica-mitchell, the last merges caused a tiny conflict, could you resolve those? This seems otherwise almost ready to go… don't hesitate to re-request review when appropriate :)

@jessica-mitchell
Copy link
Contributor Author

@terhorstd I'm still looking for someone to review the table I created that compares NEST terms with commonly used ones in research (like CUBA and psc for current-based)

doc/htmldoc/installation/nestml.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/installation/index.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@pnbabu pnbabu left a comment

Choose a reason for hiding this comment

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

Some additional comments

  • In the Lecturer installation instructions page, under the Docker image section, could you also mention that NESTML is included within the NEST docker image?
  • Can the nest::docs logo in the header be made clickable so that the user can navigate to the home page by clicking that?
  • The Contents on the right-hand side for all pages contain a link to Show Source . Is this kept deliberately?

doc/extractor_userdocs.py Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
@jessica-mitchell
Copy link
Contributor Author

Some additional comments

* In the `Lecturer installation instructions` page, under the `Docker image` section, could you also mention that NESTML is included within the NEST docker image?

Done

* Can the `nest::docs` logo in the header be made clickable so that the user can navigate to the home page by clicking that?

Yes, but that's for another PR.

* The `Contents` on the right-hand side for all pages contain a link to `Show Source` . Is this kept deliberately?

That is the default for the theme and I dont see anything wrong with it. If you think this needs to change then please create an issue for it. I don't want to add a bunch more irrelevant things to this PR or it will never be merged. The focus is on the model intro page.

Copy link
Contributor

@clinssen clinssen left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@akorgor akorgor left a comment

Choose a reason for hiding this comment

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

This already looks great and useful! :) I added some minor to medium suggestions.

doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/extractor_userdocs.py Outdated Show resolved Hide resolved
doc/extractor_userdocs.py Show resolved Hide resolved
doc/htmldoc/ref_material/glossary.rst Show resolved Hide resolved
doc/htmldoc/installation/index.rst Outdated Show resolved Hide resolved
doc/htmldoc/ref_material/glossary.rst Outdated Show resolved Hide resolved
doc/htmldoc/ref_material/glossary.rst Outdated Show resolved Hide resolved
doc/htmldoc/ref_material/glossary.rst Outdated Show resolved Hide resolved
doc/htmldoc/ref_material/glossary.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
@jessica-mitchell
Copy link
Contributor Author

@akorgor thanks for your feedback, I think I addressed all the points you raised, please resolve the remaining conversations and approve if you are satisfied

doc/htmldoc/ref_material/glossary.rst Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Show resolved Hide resolved
doc/extractor_userdocs.py Show resolved Hide resolved
@jessica-mitchell
Copy link
Contributor Author

@akorgor sorry about that! Now, I think I got them all - removed the bullet text

doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
doc/htmldoc/models/models-main.rst Outdated Show resolved Hide resolved
@jessica-mitchell
Copy link
Contributor Author

@akorgor I have added your suggestion, thanks! Can you take one final look and approve if you're happy?

@jessica-mitchell jessica-mitchell merged commit a586d8f into nest:master Dec 6, 2022
@jessica-mitchell jessica-mitchell deleted the add-intro-models branch April 17, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants