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

DM-13916: Numpydoc conversion for ap_association. #13

Merged
merged 5 commits into from Apr 10, 2018
Merged

Conversation

morriscb
Copy link
Contributor

@morriscb morriscb commented Apr 2, 2018

Create numpydocs and enable sphinx.

Debug Sphnix docs.

dia_collection.py and assoc_db_sqlite.py do not properly compile.
Clean up function Return docs.

Clean numpydocs.

Debug Sphnix docs.

dia_collection.py and assoc_db_sqlite.py do not properly compile.

Completed sphnix-doc build

Clean up numpydoc strings.
Copy link
Contributor

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

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

Thanks for fleshing out so many of those returned struct descriptions!

Handful of comments, the main ones being:

  1. Check that you have Returns instead of Return.
  2. Check that your Returns have a description of the returned value.
  3. There are some lingering merge conflict blocks (search all files for "<<<" in case I missed some).


Parameters
----------
table_name : str
table_name : `str`
Name of the new table to create

Return
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be Returns. Check this everywhere.


Returns
-------
lsst.ap.association.DIAObjectCollection
dia_objects : `lsst.ap.association.DIAObjectCollection`
Copy link
Contributor

Choose a reason for hiding this comment

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

A sentence about what this returned collection is.

@@ -369,16 +360,17 @@ def _get_dia_objects(self, indexer_indices, expMd=None):

Parameters
----------
indexer_indices : array like of ints
indexer_indices : `ndarray` of `int`s
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that array-like of `int`s is more appropriate here, since I'd bet a numpy.ndarray would also work.


Returns
-------
list of lsst.ap.association.DIAObjects
dia_objects : `list` of `lsst.ap.association.DIAObjects`
Copy link
Contributor

Choose a reason for hiding this comment

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

Description of this list.


Return
------
bool
is_contained : `bool`
Copy link
Contributor

Choose a reason for hiding this comment

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

Description.

dia_object_schema : `lsst.afw.table.Schema`
=======
`lsst.afw.table.Schema`
>>>>>>> c8d554a... Clean numpydocs.
Copy link
Contributor

Choose a reason for hiding this comment

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

You've got a lingering merge conflict stuck in here!

dia_source_schema : `lsst.afw.table.Schema`
=======
`lsst.afw.table.Schema`
>>>>>>> c8d554a... Clean numpydocs.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Another merge conflict block.

Optional input SourceRecord containing summary statistics on
the input SourceCatalog.

>>>>>>> c8d554a... Clean numpydocs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another merge conflict.

value : ``record value``
=======
``record value``
>>>>>>> c8d554a... Clean numpydocs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another merge conflict.

object.

Return
------
n_dia_sources : `int`
Copy link
Contributor

Choose a reason for hiding this comment

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

description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the return on this one because it is a property.

@morriscb morriscb merged commit df7e40d into master Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants