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.

Sign up for GitHub

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

Cleanup builtin plugins interface #5333

Merged
merged 3 commits into from
Jan 2, 2025
Merged

Cleanup builtin plugins interface #5333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bbc7fe
cleanup builtin plugins interface
brimoor Jan 1, 2025
75c481b
normalize fpc import
ritch Jan 2, 2025
c8c4ef5
fix docstring
brimoor Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/generate_docs.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,17 @@ sphinx-apidoc --force --no-toc --separate --follow-links \
--templatedir=docs/templates/apidoc \
-o docs/source/api fiftyone \
fiftyone/brain/internal/models \
fiftyone/constants \
fiftyone/internal \
fiftyone/server \
fiftyone/service \
fiftyone/management \
fiftyone/api

sphinx-apidoc --force --no-toc --separate --follow-links \
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see...

image

Hmm we haven't really figured out how to document operators/panels in a way that they provide utility in our docs.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it makes sense to add this as "API Reference" for plugins. I think the existing fiftyone.operators.operations is what I would expect in a section like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also is there any way to adjust the header/breadcrumb - it confusingly says "Docs / Plugins Overview / plugins"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's not very usable in its current form. The only reason I added this is that #5261 broke a few links in the plugins docs and release notes that used to link to (equally unhelpful) resources about the available builtin operators in the fiftyone API reference tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One possible solution is to add class docstrings to all operators/panels that describe how to use them, like I did for ViewExpression here:
https://docs.voxel51.com/api/fiftyone.core.expressions.html?highlight=viewexpression#fiftyone.core.expressions.ViewExpression

That would be a bit awkward though for this use case.

It's not that useful to have the panels in the API reference, as they aren't meant to be used programmatically and really should already have dedicated documentation sections for them.

There is some value in having a list of all the builtin operators, in the sense that they are totally undocumented to-date. It would be reasonable for some of them to have programmatic execution examples, but generally they are really intended to be invoked via the App's Operator browser. But a docstring is an awkward place to document in-App usage. What we really need is some kind of operator documentation page that shows GIFs of every single operator in use.

Copy link
Contributor

Choose a reason for hiding this comment

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

What we really need is some kind of operator documentation page that shows GIFs of every single operator in use.

Sure - I've always thought that we could allow for self-documentation, since operators define their own input/output.

We should change the "API reference". Based on what you said above I think the only value of this is the listing of what is in /plugins. In that case we should call it "Built-in plugins" or something similar.

--templatedir=docs/templates/apidoc \
-o docs/source/plugins/api plugins

# Remove symlink
unlink fiftyone/brain

Expand Down
44 changes: 29 additions & 15 deletions docs/source/cli/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,19 +830,19 @@ Tools for working with FiftyOne operators and panels.

available commands:
{list,info}
list List operators and panels that you've installed locally.
info Prints information about operators and panels that you've installed locally.
list List operators and panels that are installed locally.
info Prints information about operators and panels that are installed locally.

.. _cli-fiftyone-operators-list:

List operators
~~~~~~~~~~~~~~

List operators and panels that you've installed locally.
List operators and panels that are installed locally.

.. code-block:: text

fiftyone operators list [-h] [-e] [-d] [-o] [-p] [-n]
fiftyone operators list [-h] [-e] [-d] [-b] [-c] [-o] [-p] [-n]

**Arguments**

Expand All @@ -852,6 +852,8 @@ List operators and panels that you've installed locally.
-h, --help show this help message and exit
-e, --enabled only show enabled operators and panels
-d, --disabled only show disabled operators and panels
-b, --builtins-only only show builtin operators and panels
-c, --no-builtins only show non-builtin operators and panels
-o, --operators-only only show operators
-p, --panels-only only show panels
-n, --names-only only show names
Expand All @@ -875,15 +877,20 @@ List operators and panels that you've installed locally.

.. code-block:: shell

# Only list panels
# List non-builtin operators and panels
fiftyone operators list --no-builtins

.. code-block:: shell

# List panels
fiftyone operators list --panels-only

.. _cli-fiftyone-operators-info:

Operator info
~~~~~~~~~~~~~

Prints information about operators and panels that you've installed locally.
Prints information about operators and panels that are installed locally.

.. code-block:: text

Expand Down Expand Up @@ -1169,8 +1176,8 @@ Tools for working with FiftyOne plugins.

available commands:
{list,info,download,requirements,create,enable,disable,delete}
list List plugins that you've downloaded or created locally.
info Prints information about plugins that you've downloaded or created
list List plugins that are installed locally.
info Prints information about plugins that are installed locally.
download Download plugins from the web.
requirements Handles package requirements for plugins.
create Creates or initializes a plugin.
Expand All @@ -1183,21 +1190,23 @@ Tools for working with FiftyOne plugins.
List plugins
~~~~~~~~~~~~

List plugins that you've downloaded or created locally.
List plugins that are installed locally.

.. code-block:: text

fiftyone plugins list [-h] [-e] [-d] [-n]
fiftyone plugins list [-h] [-e] [-d] [-b] [-c] [-n]

**Arguments**

.. code-block:: text

optional arguments:
-h, --help show this help message and exit
-e, --enabled only show enabled plugins
-d, --disabled only show disabled plugins
-n, --names-only only show plugin names
-h, --help show this help message and exit
-e, --enabled only show enabled plugins
-d, --disabled only show disabled plugins
-b, --builtins-only only show builtin plugins
-c, --no-builtins only show non-builtin plugins
-n, --names-only only show plugin names

**Examples**

Expand All @@ -1216,12 +1225,17 @@ List plugins that you've downloaded or created locally.
# List disabled plugins
fiftyone plugins list --disabled

.. code-block:: shell

# List non-builtin plugins
fiftyone plugins list --no-builtins

.. _cli-fiftyone-plugins-info:

Plugin info
~~~~~~~~~~~

List plugins that you've downloaded or created locally.
List plugins that are installed locally.

.. code-block:: text

Expand Down
12 changes: 5 additions & 7 deletions docs/source/plugins/developing_plugins.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ more complex workflows such as requesting annotation of samples from a
configurable backend. Operators can even be composed of other operators or be
used to add functionality to custom panels.

FiftyOne comes with a number of builtin
:mod:`Python <fiftyone.operators.builtin.operations>` and
FiftyOne comes with a number of builtin :mod:`Python <plugins.operators>` and
`JavaScript <https://github.com/voxel51/fiftyone/blob/develop/app/packages/operators/src/built-in-operators.ts>`_
operators for common tasks that are intended for either user-facing or internal
plugin use.
Expand Down Expand Up @@ -676,7 +675,7 @@ outputs. They can be :ref:`executed <using-operators>` by users in the App or
triggered internally by other operators.

Operators can be defined in either Python or JS, and FiftyOne comes with a
number of builtin :mod:`Python <fiftyone.operators.builtin>` and
number of builtin :mod:`Python <plugins.operators>` and
`JS <https://github.com/voxel51/fiftyone/blob/develop/app/packages/operators/src/built-in-operators.ts>`_
operators for common tasks.

Expand Down Expand Up @@ -1405,11 +1404,10 @@ method via :meth:`ctx.ops <fiftyone.operators.operations.Operations>` and
:meth:`ctx.trigger <fiftyone.operators.executor.ExecutionContext.trigger>`.

The :meth:`ctx.ops <fiftyone.operators.operations.Operations>` property of an
execution context exposes all builtin
:mod:`Python <fiftyone.operators.builtin>` and
execution context exposes all builtin :mod:`Python <plugins.operators>` and
`JavaScript <https://github.com/voxel51/fiftyone/blob/develop/app/packages/operators/src/built-in-operators.ts>`_
in a conveniently documented functional interface. For example, many operations
involve updating the current state of the App:
operators in a conveniently documented functional interface. For example, many
operations involve updating the current state of the App:

.. code-block:: python
:linenos:
Expand Down
1 change: 1 addition & 0 deletions docs/source/plugins/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,5 @@ App? Just open the :ref:`Embeddings panel <app-embeddings-panel>`:
Overview <self>
Using plugins <using_plugins>
Developing plugins <developing_plugins>
API reference <api/plugins>
TypeScript API reference <ts-api>
6 changes: 3 additions & 3 deletions docs/source/release-notes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,13 @@ SDK
- Improved support for creating summary fields and indexes
`#5091 <https://github.com/voxel51/fiftyone/pull/5091>`_
- Added support for creating compound indexes when using the builtin
:class:`create_index <fiftyone.operators.builtin.CreateIndex>` operator that
:class:`create_index <plugins.operators.CreateIndex>` operator that
optimize sidebar queries for group datasets
`#5174 <https://github.com/voxel51/fiftyone/pull/5174>`_
- The builtin
:class:`clear_sample_field <fiftyone.operators.builtin.ClearSampleField>`
:class:`clear_sample_field <plugins.operators.ClearSampleField>`
and
:class:`clear_frame_field <fiftyone.operators.builtin.ClearFrameField>`
:class:`clear_frame_field <plugins.operators.ClearFrameField>`
operators now support clearing fields of views, in addition to full datasets
`#5122 <https://github.com/voxel51/fiftyone/pull/5122>`_
- Fixed a bug that prevented users with `pydantic` installed from loading the
Expand Down
2 changes: 1 addition & 1 deletion docs/source/user_guide/evaluation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ When you evaluate a model in FiftyOne, you get access to the standard aggregate
metrics such as classification reports, confusion matrices, and PR curves
for your model. In addition, FiftyOne can also record fine-grained statistics
like accuracy and false positive counts at the sample-level, which you can
:ref:`interactively explore <model-evaluation-panel>` in the App to diagnose
:ref:`interactively explore <app-model-evaluation-panel>` in the App to diagnose
the strengths and weaknesses of your models on individual data samples.

Sample-level analysis often leads to critical insights that will help you
Expand Down
77 changes: 63 additions & 14 deletions fiftyone/core/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2905,7 +2905,7 @@ def execute(parser, args):


class OperatorsListCommand(Command):
"""List operators and panels that you've installed locally.
"""List operators and panels that are installed locally.

Examples::

Expand All @@ -2918,7 +2918,10 @@ class OperatorsListCommand(Command):
# List disabled operators and panels
fiftyone operators list --disabled

# Only list panels
# List non-builtin operators and panels
fiftyone operators list --no-builtins

# List panels
fiftyone operators list --panels-only
"""

Expand All @@ -2938,12 +2941,26 @@ def setup(parser):
default=None,
help="only show disabled operators and panels",
)
parser.add_argument(
"-b",
"--builtins-only",
action="store_true",
default=None,
help="only show builtin operators and panels",
)
parser.add_argument(
"-c",
"--no-builtins",
action="store_true",
default=None,
help="only show non-builtin operators and panels",
)
parser.add_argument(
"-o",
"--operators-only",
action="store_true",
default=None,
help="only show operators",
help="only show non-panel operators",
)
parser.add_argument(
"-p",
Expand All @@ -2968,18 +2985,25 @@ def execute(parser, args):
else:
enabled = "all"

if args.builtins_only:
builtin = True
elif args.no_builtins:
builtin = False
else:
builtin = "all"

if args.operators_only:
type = "operator"
elif args.panels_only:
type = "panel"
else:
type = None

_print_operators_list(enabled, type, args.names_only)
_print_operators_list(enabled, builtin, type, args.names_only)


def _print_operators_list(enabled, type, names_only):
operators = foo.list_operators(enabled=enabled, type=type)
def _print_operators_list(enabled, builtin, type, names_only):
operators = foo.list_operators(enabled=enabled, builtin=builtin, type=type)

if names_only:
operators_map = defaultdict(list)
Expand Down Expand Up @@ -3016,7 +3040,7 @@ def _print_operators_list(enabled, type, names_only):


class OperatorsInfoCommand(Command):
"""Prints info about operators and panels that you've installed locally.
"""Prints info about operators and panels that are installed locally.

Examples::

Expand Down Expand Up @@ -3498,7 +3522,7 @@ def execute(parser, args):


class PluginsListCommand(Command):
"""List plugins that you've installed locally.
"""List plugins that are installed locally.

Examples::

Expand All @@ -3510,6 +3534,9 @@ class PluginsListCommand(Command):

# List disabled plugins
fiftyone plugins list --disabled

# List non-builtin plugins
fiftyone plugins list --no-builtins
"""

@staticmethod
Expand All @@ -3528,6 +3555,20 @@ def setup(parser):
default=None,
help="only show disabled plugins",
)
parser.add_argument(
"-b",
"--builtins-only",
action="store_true",
default=None,
help="only show builtin plugins",
)
parser.add_argument(
"-c",
"--no-builtins",
action="store_true",
default=None,
help="only show non-builtin plugins",
)
parser.add_argument(
"-n",
"--names-only",
Expand All @@ -3544,11 +3585,18 @@ def execute(parser, args):
else:
enabled = "all"

_print_plugins_list(enabled, args.names_only)
if args.builtins_only:
builtin = True
elif args.no_builtins:
builtin = False
else:
builtin = "all"

_print_plugins_list(enabled, builtin, args.names_only)


def _print_plugins_list(enabled, names_only):
plugin_defintions = fop.list_plugins(enabled=enabled)
def _print_plugins_list(enabled, builtin, names_only):
plugin_defintions = fop.list_plugins(enabled=enabled, builtin=builtin)

if names_only:
for pd in plugin_defintions:
Expand All @@ -3558,14 +3606,15 @@ def _print_plugins_list(enabled, names_only):

enabled_plugins = set(fop.list_enabled_plugins())

headers = ["plugin", "version", "enabled", "directory"]
headers = ["plugin", "version", "enabled", "builtin", "directory"]
rows = []
for pd in plugin_defintions:
rows.append(
{
"plugin": pd.name,
"version": pd.version or "",
"enabled": pd.name in enabled_plugins,
"enabled": pd.builtin or pd.name in enabled_plugins,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍

"builtin": pd.builtin,
"directory": pd.directory,
}
)
Expand All @@ -3577,7 +3626,7 @@ def _print_plugins_list(enabled, names_only):


class PluginsInfoCommand(Command):
"""Prints info about plugins that you've installed locally.
"""Prints info about plugins that are installed locally.

Examples::

Expand Down
Loading
Loading