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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docs for config system and presets #1150

Merged
merged 2 commits into from
Aug 26, 2016
Merged

Add docs for config system and presets #1150

merged 2 commits into from
Aug 26, 2016

Conversation

tbekolay
Copy link
Member

@tbekolay tbekolay commented Aug 25, 2016

Description:
This adds a new page to the user docs explaining the config system and the presets (currently just the ThresholdingEnsembles preset). Note that this tries not to overlap too much with the config.ipynb notebook, but links to it instead.

Motivation and context:
We discussed at the last dev meeting that these should be added to the docs before the 2.2.0 release.

How has this been tested?
I've built the docs with these changes. They build without warnings and look pretty deece.

Where should a reviewer start?
Files changed tab.

How long should this take to review?

  • Average (neither quick nor lengthy)

Types of changes:

  • Bug fix (non-breaking change which fixes an issue) -- sort of

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly. 馃槈
  • I have included a changelog entry.
  • I have added tests to cover my changes. (nothing to test)
  • All new and existing tests passed.

The whitespace rule for <code> blocks was causing newlines in
Markdown cell code listings to disappear, which looked bad.
This was most apparent in the usage/config example.
@Seanny123
Copy link
Contributor

LGTM.

@jgosmann
Copy link
Collaborator

I found one sentence that I had trouble with understanding. I rewrote it and made it a bit more specific regarding the usage of object vs. class. I also moved the config documentation behind the network documentation because that seems to me like a more logical progression (especially because the config documentation mentions networks in a few places).

One thing that slightly irritates me is that all the headings in the same level of the navigation are nouns (e.g. "Networks"), where this one title is a whole phrase ("Setting parameters with Configs"). But shortening it to "Configs" or "Configurations" makes it less expressive.

So: LGTM 馃嵃

@tbekolay
Copy link
Member Author

Reword looks good, but I'd like to revert the move from before network docs to after. The network object itself is documented in frontend_api.rst, which comes first; the networks.rst doc only documents the specific networks that are included with Nengo. In terms of abstraction levels, I think presets are lower level than networks because networks use presets, and not the other way around, so I would put them first.

One thing that slightly irritates me is that all the headings in the same level of the navigation are nouns (e.g. "Networks"), where this one title is a whole phrase ("Setting parameters with Configs").

This is a good point... I've been trying to think of a noun version of that title and nothing is popping out at me. Could switch it to just be "The config system" and rearrange the headings a bit... alternatively, can change the headings of the other sections to be more free-form. I think probably this is good enough for now, can revisit later.

@tbekolay tbekolay assigned tbekolay and unassigned tbekolay Aug 26, 2016
@tbekolay
Copy link
Member Author

Pushed commits to revert and fix a line that was too long; if you're okay with it this way @jgosmann please squash and merge! 馃憤 馃寛 馃憤

@jgosmann
Copy link
Collaborator

jgosmann commented Aug 26, 2016

The network object itself is documented in frontend_api.rst, which comes first; the networks.rst doc only documents the specific networks that are included with Nengo.

Right!

This involved updating the docstrings for Config, ClassParams,
and InstanceParams, as well as modifying the documentation itself.
The config example reference was moved from the network section
to the new config section.

I actually think that this tells a pretty decent story now, as the
config system is well justified by the frontend API, which justifies
the use of pre-built networks, since you see how complicated they
can get when you deal with the cascading defaults system.
@jgosmann jgosmann self-assigned this Aug 26, 2016
@jgosmann jgosmann merged commit f5279f1 into master Aug 26, 2016
@jgosmann jgosmann deleted the doc-presets branch August 26, 2016 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants