Documentation

[jgosmann]

Motivation and context:
This PR is adding documentation to Nengo SPA and work in progress. Comments, improvements, and help are welcome as I go along.

Interactions with other PRs:

How has this been tested?

How long should this take to review?

• Quick (less than 40 lines changed or changes are straightforward)
• Average (neither quick nor lengthy)
• Lengthy (more than 150 lines changed or changes are complicated)

Where should a reviewer start?

Types of changes:

• Non-code change (touches things like tests, documentation, build scripts)

Checklist:

• I have read the CONTRIBUTING.rst document.
• I have updated the documentation accordingly.
• I have included a changelog entry.
• [n/a] I have added tests to cover my changes.
• All new and existing tests passed.

Still to do:

• User Guide
• Examples
• API documentation
• proof read, including checking cross references
• add example of adding custom module

[Seanny123]

@jgosmann could you rebase this to master, so I can start updating the documentation? I would do it myself, but I'm afraid I might do it wrong. It needs to be rebased, because otherwise it tries to import memoize which doesn't exist anymore.

[jgosmann]

rebased and force pushed

[Seanny123]

The Thalamus module has a really weird looking docstring:

    def construct_channel(
            self, target_net, target_input, net, label=None):
        """Construct a channel.

        Channels are an additional neural population in-between a source
        population and a target population. This allows inhibiting the channel
        without affecting the source and thus is useful in routing information.

        Parameters
        ----------
        target_net : :class:`spa.Network`
            The network that the channel will project to.
        target_input : :class:`spa.Vocabulary`
            
        net : :class:`nengo.Network`, optional
            Network to which to add the channel. Defaults to ``self.spa``.
        label : str, optional
            Label for the channel.

        Returns
        -------
        :class:`nengo.networks.EnsembleArray`
            The constructed channel.
        """

What does :class: do and do we want to include it in our docstrings?

On the same topic, do we want to document objects as being from nengo_spa or spa? For example, if one of the parameters of a function is a Vocabulary instance, should this be documented as spa.Vocabulary or nengo_spa.Vocabulary or just Vocabulary?

[Seanny123]

Note: my latest push is going to break Travis because it depends on the Basal Ganglia accessor PR.

[jgosmann]

Shall one of us rebase this branch to master? (Otherwise Travis-CI will spam me with an email for every commit.)

[Seanny123]

Yes please. If you get this one, I promise to squash all my gross commits before you need to review.

…

On 8 Nov 2017 22:32, "Jan Gosmann" ***@***.***> wrote: Shall one of us rebase this branch to master? (Otherwise Travis-CI will spam me with an email for every commit.) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1Haf_OhPbxprHmw4flPCJE9xMx2558ks5s0nI9gaJpZM4OycLb> .

[jgosmann]

Do you want me to do it? You should have pushed all your work beforehand because otherwise you might lose it (though it is hard to lose it completely in git) or merging might be very annoying.

[Seanny123]

Yep. I've pushed all my work.

…

On 8 Nov 2017 22:38, "Jan Gosmann" ***@***.***> wrote: Do you want me to do it? You should have pushed all your work beforehand because otherwise you might lose it (though it is hard to lose it completely in git) or merging might be very annoying. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1HaZ6p8-fNbWO9Xu0XaXEYOVNDeaaoks5s0nO2gaJpZM4OycLb> .

[jgosmann]

Force pushed the rebased version. No conflicts whatsoever. 😏

[Seanny123]

Now that the new syntax has been merged, should this be rebased yet again?

[jgosmann]

Yeah

[jgosmann]

Has been rebased to master.

[jgosmann]

@bjkomer I updated the introduction coming from legacy SPA to the new syntax in this PR. Might be of interest to you.

[jgosmann]

Actually Nengo does not install the examples.¹ Not sure why zip_safe=False was set, but it can't because of the examples. zip_safe=False has to be used if a Python package contains files that need to be accessible as actual files in the file system. Without, the Python package may be installed as an egg which is basically a zip file with all the contents (thus, the files cannot be directly accessed, but need to be unzipped when read).

¹: This means that you cannot test the examples of in an installed Nengo package. While I think that we want the tests in general to be available in the install for testing backends, the tests are probably not essential (all the functionality should be tested by the other tests; testing the examples is mainly to ensure the examples are up to date and work with the current Nengo version).

For Nengo SPA I'm leaning towards not including any documentation, examples, or tests in the install. Most people will access the documentation and examples online anyways. We can add a download link to the documentation (which will be hosted on Github and Github provides links to download the branch of a repository as zip). This would then include the examples; the Jupyter notebook files can be downloaded from within the documentation. The tests are only required for development and in that case one is working on a git clone anyways.

[Seanny123]

I agree with the direction you're leaning in.

…

On Wed, Apr 18, 2018 at 5:14 PM, Jan Gosmann ***@***.***> wrote: Actually Nengo does not install the examples.¹ Not sure why zip_safe=False was set, but it can't because of the examples. zip_safe=False has to be used if a Python package contains files that need to be accessible as actual files in the file system. Without, the Python package may be installed as an egg which is basically a zip file with all the contents (thus, the files cannot be directly accessed, but need to be unzipped when read). ¹: This means that you cannot test the examples of in an installed Nengo package. While I think that we want the tests in general to be available in the install for testing backends, the tests are probably not essential (all the functionality should be tested by the other tests; testing the examples is mainly to ensure the examples are up to date and work with the current Nengo version). For Nengo SPA I'm leaning towards not including any documentation, examples, or tests in the install. Most people will access the documentation and examples online anyways. We can add a download link to the documentation (which will be hosted on Github and Github provides links to download the branch of a repository as zip). This would then include the examples; the Jupyter notebook files can be downloaded from within the documentation. The tests are only required for development and in that case one is working on a git clone anyways. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1Haenltb1vlg7gok8Dtq2PcHRvAtXxks5tp9b9gaJpZM4OycLb> .

[jgosmann]

Excluded examples, tests, docs from install. Fixed the tests (I think, we'll see what Travis and AppVeyor think). Should be ready for final review ...

[jgosmann]

Oh, and AppVeyor is working again. It didn't like the rename of the repository from nengo_spa to nengo-spa.

[Seanny123]

New changes LGTM. Are we going to wait for a review of @tcstewar before merging?

[jgosmann]

Probably not, except @tcstewar is exceptionally fast. (We can always make changes to the documentation later, and it is an improvement on the current state.)

[jgosmann]

Added one more commit because a test was failing after squashing the fixup commits. @Seanny123 still looks good to you?

[Seanny123]

Yep, still good.

…

On Fri, Apr 20, 2018, 10:49 Jan Gosmann, ***@***.***> wrote: Added one more commit because a test was failing after squashing the fixup commits. @Seanny123 <https://github.com/Seanny123> still looks good to you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1HaWOn3talNANx0owkZvwhmKWN8Iasks5tqgORgaJpZM4OycLb> .