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

Discussion about null vs [] (for arrays and objects in task arguments) #209

Open
tcompa opened this issue Jun 23, 2023 · 2 comments
Open
Labels
JSON Schemas Editing of WorkflowTask arguments based on JSON Schemas to be discussed Not ready to implement

Comments

@tcompa
Copy link
Collaborator

tcompa commented Jun 23, 2023

Refs:

Let's try to better define the issue.
Note that we'll likely start with a workaround, and then we'll have to make an effort to define the scope of this feature.

The problem in a nutshell:

  1. For a Python function (say this one https://github.com/fractal-analytics-platform/fractal-tasks-core/blob/main/fractal_tasks_core/tasks/copy_ome_zarr.py), having ROI_table_names=None or ROI_table_names=[] are both valid options, and the internal function logic leads to two different behaviors. Note that the Python flexibility is coming from the Optional type hint.
  2. It's unclear how one could set apart the two fractal-web, since the only options are add/delete entries. Note that the schema is not aware that the argument can be null. The schema only states that
    • That argument is not required
    • If it is there, it must be a (possibly empty) list of strings

Let's start with a minimal schema

{
        "title": "CopyOmeZarr",
        "type": "object",
        "properties": {
            "ROI_table_names": {
            "title": "Roi Table Names",
            "type": "array",
            "items": { "type": "string" },
            "description": "My description."
          }
        },
        "required": [ ],
        "additionalProperties": false
      }

which comes from a Python-function definition like

def function(ROI_table_names: Optional[Sequence[str]] = None):
    ...

Since the ROI_table_names property in the schema has no default, fractal-server will not set it to anything in WorkflowTask.args. This means that we start with

args = {}

If we insert the schema in https://rjsf-team.github.io/react-jsonschema-form (which appears to support much more of the JSON Schema flexibility than what we may aim for), the default formData is

args = {
   "ROI_table_names": []
}

This is the same behavior which is currently implemented in fractal-web. The reason for setting this to an empty array by default, even if it's not defined like that in the schema, is that it then allows the user to add/remove items. If we were to let it unset, as in args = {}, the user would not be able to add/remove items (*).

At this point, the user can add/remove items at will. If they remove all items, the property will be set to [] (which is perfectly valid value, as per the schema). Upon saving, this value will be sent to fractal-server, and will be set in WorkflowTask.args. Up to here, nothing weird.

QUESTION 1: how would the user be able to set ROI_table_names=null?
QUESTION 2: how could the user tell the web client to remove ROI_table_names from the args object that will be sent to fractal-server?

Note that the two questions are the same, because we are removing null values from args before calling the PATCH endpoint.

As far as I understand, the answer to Q1 and Q2 in https://rjsf-team.github.io/react-jsonschema-form is "this is not possible".
In fractal-web we could make it possible, but then we'd hit Q3: "how would the user be able to set ROI_table_names=[].

TLDR: we cannot expose the full flexibility of the Python function through the schema-based form, and we should take decisions like:

  • Is there a way to expose this?
  • Should we impose constraints on python functions?
  • ...

The answer is clearly non-trivial..


(*) Alternative possibility: we do let it unset, but when the user clicks "add item to array" we first check if the key ROI_table_names exists in args. If it does, then we append the new item. If it doesn't, then we first create an empty array and then proceed. This, however, only fixes half of the problem and it is not to be considered an actual solution.

@tcompa
Copy link
Collaborator Author

tcompa commented Jun 23, 2023

Side-comment:

It's likely that slightly different schemas (as the v2 example in fractal-analytics-platform/fractal-tasks-core#375 (comment)) may allow handling this problem slightly differently, because they encode information which is much closer to the Optional keyword. But that's not relevant for the moment, since we are not moving to Pydantic v2 soon.

rkpasia added a commit that referenced this issue Jun 23, 2023
@tcompa
Copy link
Collaborator Author

tcompa commented Jun 26, 2023

Re-opening this one, in view of a broader discussion (not to be had today!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON Schemas Editing of WorkflowTask arguments based on JSON Schemas to be discussed Not ready to implement
Projects
Development

No branches or pull requests

1 participant