-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
OPF Model docstrings #3589
OPF Model docstrings #3589
Conversation
@@ -69,6 +70,7 @@ def requireAnomalyModel(func): | |||
""" | |||
Decorator for functions that require anomaly models. | |||
""" | |||
@wraps(func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, we lose the original docstrings on introspection, and Sphinx cannot get to them.
.. automethod:: nupic.frameworks.opf.htm_prediction_model.HTMPredictionModel.getAnomalyParameter(param) | ||
.. automethod:: nupic.frameworks.opf.htm_prediction_model.HTMPredictionModel.anomalyRemoveLabels(start, end, labelFilter) | ||
.. automethod:: nupic.frameworks.opf.htm_prediction_model.HTMPredictionModel.anomalyAddLabel(start, end, labelName) | ||
.. automethod:: nupic.frameworks.opf.htm_prediction_model.HTMPredictionModel.anomalyGetLabels(start, end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to include these manually because they are decorated functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird. That's good to know
@@ -329,6 +342,9 @@ def getAnomalyParameter(self, param): | |||
def anomalyRemoveLabels(self, start, end, labelFilter): | |||
""" | |||
Remove labels from the anomaly classifier within this model. | |||
|
|||
:param start: (int) index to start removing labels | |||
:param end: (int) index to end removing labels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shit I forgot labelFilter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. There are small RST formatting issue, but otherwise it's great. I like the OPF docs.
If underlying implementation does not support min/max stats collection, | ||
or if a field type does not support min/max (non scalars), the return | ||
value will be None. | ||
|
||
:param fieldName: (string) name of field to get max |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be: to get min
src/nupic/data/record_stream.py
Outdated
""" | ||
:returns: (list) of :class:`nupic.data.fieldmeta.FieldMetaInfo`objects for | ||
each field in the stream. Might be None, if that information is provided | ||
externally (thru stream def, for example). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space missing between :class:nupic.data.fieldmeta.FieldMetaInfo
and objects
, so the formatting is off in the HTML.
Super minor: consider spelling thru
-> through
.
src/nupic/data/record_stream.py
Outdated
return _getFieldIndexBySpecial(self.getFields(), FieldMetaSpecial.category) | ||
|
||
|
||
def getLearningFieldIdx(self): | ||
""" Return index of the 'learning' field. """ | ||
""" | ||
:returns: (int) index of the ``learning ``field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
space is in the wrong spot. Should be learning
field.
|
||
:param modelConfig: (dict) | ||
A dictionary describing the current model, | ||
`described here <../../quick-start/example-model-params.html>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
:param inferenceType: (:class:`nupic.frameworks.opf.opf_utils.InferenceType`) | ||
:param encoders: a dict of dicts, eventually sent to | ||
:meth:`~nupic.encoders.multi.MultiEncoder.addMultipleEncoders` (see | ||
there for details). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean, see method docs? (addMultipleEncoders
) Or is there a missing hyperlink?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant see the addMultipleEncoders
method docs for complete description of this parameter. Will make it clearer.
I made changes in 1bf8894 that don't seem to be reflected on the files page of this PR. Not sure why. |
fixes #3588