GeezerVison is showing github.com — [pyos][docs] API Docs missing large amount of user-facing functions/methods/classes · Issue #410 · python-graphblas/python-graphblas
[pyos][docs] API Docs missing large amount of user-facing functions/methods/classes #410DescriptionAs-is, only the top-level classes are included in the API documentation, which seems to leave out most of the user-facing functions if i'm not mistaken?
Eg. I was trying to figure out how I would know about the various classes/functions used in the example notebooks, like i saw binary.truediv in the pagerank example, and there is nothing like that in the API docs.
It seems like the place these are documented are here (for operators): https://python-graphblas.readthedocs.io/en/stable/user_guide/operators.html but that list is hardcoded and so misses a decent amount of the available binary operators (table at bottom of issue).
Ideally these would all be listed in the API docs, which I think would require a refactor of the docs ( #409 ).
Since this package is a wrapper of a few different packages, it doesn't seem practical or useful to document every single one of these functions/classes, since presumably the wrapped libraries have their own documentation. It should be possible to list them and link to the original docs, however, and that seems like a pretty necessary thing to have in place to make this broadly usable by people who don't already know GraphBLAS from other contexts (I'm assuming these are all standard across GraphBLAS distributions?).
Aside from the wrapped functions/classes, there are a large number of functions/classes that lack a docstring (351 by an inspection that also includes trivially missing docstrings like module-level docstrings. You all will have a better sense of what counts as "user-facing" in this context, but presumably some subset of those are intended for use by the user.
From a developer/contributor POV, this additional documentation would be necessary for me to contribute to the package, as currently I am trying to make sense of the structure of the package, and some narrative docs in, for example, the positively massive core/operator.py file would go a long way in helping me understand that. Again, understanding this is a wrapper package which requires a lot of metaprogramming, so some documentation concerns don't apply since the functions/methods are generated dynamically, but as is I don't see a way for me to understand how that metaprogramming works. Perhaps that should be a separate issue though.
dir(graphblas.binary) |
docs |
| 'absfirst' |
|
| 'abssecond' |
|
| 'any' |
|
| 'atan2' |
'atan2' |
| 'band' |
'band' |
| 'bclr' |
|
| 'bget' |
|
| 'binom' |
|
| 'bor' |
'bor' |
| 'bset' |
|
| 'bshift' |
|
| 'bxnor' |
'bxnor' |
| 'bxor' |
'bxor' |
| 'cdiv' |
|
| 'cmplx' |
|
| 'copysign' |
|
| 'eq' |
'eq' |
| 'first' |
'first' |
| 'firsti' |
|
| 'firsti1' |
|
| 'firstj' |
|
| 'firstj1' |
|
| 'floordiv' |
|
| 'fmod' |
'fmod' |
| 'from_string' |
|
| 'ge' |
'ge' |
| 'gt' |
'gt' |
| 'hypot' |
|
| 'isclose' |
|
| 'iseq' |
|
| 'isge' |
|
| 'isgt' |
|
| 'isle' |
|
| 'islt' |
|
| 'isne' |
|
| 'land' |
'land' |
| 'ldexp' |
|
| 'le' |
'le' |
| 'lor' |
'lor' |
| 'lt' |
'lt' |
| 'lxnor' |
'lxnor' |
| 'lxor' |
'lxor' |
| 'max' |
'max' |
| 'min' |
'min' |
| 'minus' |
'minus' |
| 'ne' |
'ne' |
| 'numpy' |
|
| 'pair' |
'pair' |
| 'plus' |
'plus' |
| 'pow' |
'pow' |
| 'rdiv' |
|
| 'register_anonymous' |
|
| 'register_new' |
|
| 'remainder' |
|
| 'rfloordiv' |
|
| 'rminus' |
|
| 'rpow' |
|
| 'rtruediv' |
|
| 'second' |
'second' |
| 'secondi' |
|
| 'secondi1' |
|
| 'secondj' |
|
| 'secondj1' |
|
| 'ss' |
|
| 'times' |
'times' |
| 'truediv' |
'truediv' |
Related to: #409
Part of: pyOpenSci/software-submission#81 Activityeriknw commented on Mar 20, 2023 I agree it would be great to more thoroughly document the operators such as from gb.binary. We can/should probably even group them as "builtin, part of the spec", "builtin, SuiteSparse:GraphBLAS-specific", and UDF. We can include a lot of relavent information in a table view for the different operators, and we can even indicate what the numpy-equivalent is when possible (since we have this info in e.g. gb.binary.numpy namespace). Whatever we do though would need to be autogenerated.
Aside from the wrapped functions/classes, there are a large number of functions/classes that lack a docstring (351 by an inspection that also includes trivially missing docstrings like module-level docstrings. You all will have a better sense of what counts as "user-facing" in this context, but presumably some subset of those are intended for use by the user.
A few of them, yes, but it's not quite as dire as it looks at a glance. I commented out the following ignore codes and ran ruff via pre-commit run --all-files:
|
ignore = [ |
|
# Would be nice to fix these |
|
"D100", # Missing docstring in public module |
|
"D101", # Missing docstring in public class |
|
"D102", # Missing docstring in public method |
|
"D103", # Missing docstring in public function |
|
"D104", # Missing docstring in public package |
|
"D105", # Missing docstring in magic method |
This probably has similar results to your inspection. When looking through the results, I see very little that I regard as user-facing or particularly important. It also doesn't recognize that @wrapdoc decorator adds a docstring. To be sure, though, there are a few user-facing methods and properties such as .new(...) that could use docstrings, and we can do better in general. If you see any specific methods, functions, properties, or error messages that should be added or improved during the course of your experiments, please comment here or raise new issues :)
the positively massive core/operator.py file
Heh, every time I've tried to split it up, I've failed. Here's a quick sketch of the important bits: we call e.g. UnaryOp._initialize, which then does a regex on lib (the objects from cffi that wraps the C implementation) to find unary operators, then we add all the typed unary operators (as TypedBuiltinUnaryOp) to UnaryOp, which we save to gb.unary. We do the same thing for binary operators, monoids, semirings, and indexunaryops. Reacted by Szabolcs Horvát eriknw commented on Mar 27, 2023 @sneakers-the-rat, please let us know if there is anything we can do (chat, meet, reply to issues, etc.) to help you understand our project and perform the review more effectively. szhorvat commented on Jun 25, 2023 Most of these can be looked up in the GraphBLAS spec/API, as Erik mentioned, but I'd also have liked to see them directly in the python-graphblas docs.
However, there are things which are specific to this package and are not in the docs, such as the viz namespace.
io.draw is documented, but it shows a warning saying that it's deprecated in favour of viz.draw. viz.draw does not appear in the API docs or guide.
Reacted by Sultan Orazbayev sneakers-the-rat commented on Jun 27, 2023 So I actually just tried to make this work for an hour or so using autodoc.
operator methods are all present in the dir(operator) call, but also each of the operator modules overrides __dir__, so that might be something.
I tried the following automodule commands:
.. _operator_methods:
Operator Methods
----------------
Binary
~~~~~~~
.. automodule:: graphblas.binary
:members:
:undoc-members:
:imported-members:
:private-members:
numpy
~~~~~
.. automodule:: graphblas.binary.numpy
:members:
:undoc-members:
:imported-members:
:private-members:
ss
~~
.. automodule:: graphblas.binary.ss
:members:
:undoc-members:
:imported-members:
:private-members:
op
~~
.. automodule:: graphblas.core.operator.binary
:members:
:undoc-members:
:imported-members:
:private-members:
And only the last one gave a sort of partial documentation here:
(zip of the generated html file, it's missing all the stylesheets and whatnot, but the content of the html is what's important)
operator_methods.html.zip
So it seems to get some of them, but some of the methods are also in the _delayed module-level dict, and I ran out of time to chase down the other indirection methods.
This is I think related to both
#433
and
#429
The thing that is making this difficult is that I'm still not sure if there is a single source of truth for all the available operator names. I understand that they are generated at import time, but it should be possible for me to, say, call binary.list_operators() or something and get a definitive list of all possible operators, whether that be string representations, the actual objects, or both. As is, calling dir() or using tab completions is suboptimal since dir() returns all the globals as well, and tab completion requires you to hunt around in dir.
If there is such a thing, then documenting all available directives would just be a matter of making a new sphinx directive that calls autoclass for each of the operators. Or, if that's out of reach, simply pprinting the result of that function in the docs would do.
It is a small thing relative to the scale of the whole package, but it actually is a pretty important thing from a user discovery POV, and I feel like it shouldn't be too hard to implement by the devs who understand the abstraction of how the operators are registered better than I do Reacted by Sultan Orazbayev eriknw commented on Jul 11, 2023 Awesome, thanks for taking the time to explore this! This looks great (up until it stopped working) and is a good start.
I like the idea of having a definitive set of names of default operators. __dir__ gets us close, and the following should work for all the operator types:
{x for x in dir(gb.unary) if not x.startswith('_')} - {'register_new', 'register_anonymous', 'ss', 'from_string', 'numpy'}I think there probably should be a global such as DEFAULT_NAMES or whatever. (It's late, and I'm bad at coming up with names when it's late)
The discussion from this issue probably has a few action items. We should probably enumerate them and/or make new issues as appropriate. Reacted by Jonny Saunders
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|