Skip to content

Fix various typos in the documentation#2787

Merged
terhorstd merged 31 commits intonest:masterfrom
anthonyylee:master
Jun 27, 2023
Merged

Fix various typos in the documentation#2787
terhorstd merged 31 commits intonest:masterfrom
anthonyylee:master

Conversation

@anthonyylee
Copy link
Contributor

@anthonyylee anthonyylee commented May 13, 2023

Various on going fix suggestions.

@anthonyylee anthonyylee reopened this May 17, 2023
Correct and not a a typo. Apologies for the misunderstanding.
@anthonyylee anthonyylee changed the title Update part_3_connecting_networks_with_synapses.rst Fix suggestions May 17, 2023
The parameter `V_m` should not be used as a string literal when setting its values.
`np.arange(a, b)` includes `a` but excludes `b`, hence b needs to incremented to `b+1` to include `b` in the range.

`np.arange(1, N+1)` would yield an error in mismatched lengths of the two arguments of `plt.xticks()`.
Incorrect variable and template `[node id]` was not filled in with the NEST v2.x returned global-id variable, `gid`.
`print_all` is not an attribute of `SynapseCollection` object, instead `print_full` is.
The previous section (e.g., fixed indegree) used `A` and `B` to refer to the pre- and post-synaptic population respectively.

Hence, update the variable usage to match previous section's convention thus aligning with the example code given.
`syn_spec` is pointing to the previous codeblock's `CollocatedSynapses` object, whereas `syn_spec_dict` points to the immediate `CollocatedSynapses` object.
@jessica-mitchell jessica-mitchell added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels May 24, 2023
@heplesser
Copy link
Contributor

@aoot Thank you for your contribution! May I ask you to fill in the NEST Copyright Transfer Agreement and so send it to info@nest-initiative.org?

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Just a formal block until the CTA is in place.

@heplesser
Copy link
Contributor

@aoot The vale test currently fails due to an incorrect build configuration. This is fixed in master. Could you pull master into your branch?

@anthonyylee
Copy link
Contributor Author

Just pulled the master into my fork. Thanks!

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Formalities are ok, approving.

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Thanks @aoot for contributing to our documentation. It's very appreciated. I just have one comment that needs addressing.

@jessica-mitchell jessica-mitchell changed the title Fix suggestions Fix various typos in the documentation Jun 2, 2023
Copy link
Member

@nicolossus nicolossus left a comment

Choose a reason for hiding this comment

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

@aoot Thanks for the contribution. I have some small comments. I also agree with @jessica-mitchell that numpy.foo instead of np.foo should be used to be explicit.

anthonyylee and others added 8 commits June 14, 2023 08:24
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
For clarity, using the numpy namespace because the documentation does not include explicit import as statement.
The unit should be nano-siemens instead of nano-seconds.
@jessica-mitchell
Copy link
Contributor

@aoot could you please resolve the conflicts and commit the suggestion from @nicolossus? Once that's sorted we can probably merge this PR, thanks :)

@anthonyylee
Copy link
Contributor Author

Thank you for your patience! I've resolved the conflicts.

Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
@terhorstd terhorstd merged commit 10b88a2 into nest:master Jun 27, 2023
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: Maintenance Work to keep up the quality of the code and documentation.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants

Comments