Feat task

[jwong-nd]

Code for #2

[bruno-f-cruz]

• Removes Modifiable Attr Tag as this causes an error in serialization. This information is not lost, as all Literals are marked as ‘const’ in Model.model_json_schema(). All fields not marked as Literal/const may be implied as modifiable.

Can you give me an example of this? trying to reproduce it on my end. I am ok with using const but make sure @hanhou is also ok with following this standard as a way to provide metadata to his GUI. There is a big different between using const or the ModifiableAttr, since the latter does not afford validation by the json-schema standard.

• Removes multiple inheritance as this does not apply AindBehaviorModel constraints onto TaskParameters. To achieve deserialization w/ TaskParameters and BaseModel validation checks, this can be streamlined shown here.

I dont think TaskParameters cant inherit from a model that has forbid in the extra property. Try the example I sent, i am pretty sure it will fail the roundtrip if i am understand correctly

[jwong-nd]

@brunocruz, yes I tried the example you sent but the BaseModel validation is not applied to the task parameters and you could make random changes to these parameters that do not follow the schema

[jwong-nd]

@bruno-f-cruz how about this?

[bruno-f-cruz]

@brunocruz, yes I tried the example you sent but the BaseModel validation is not applied to the task parameters and you could make random changes to these parameters that do not follow the schema

Not sure I follow. The model will still be validated. The extra properties will be added as dict keys. If we don't do this, we won't be able to round trip against the base task class. This is kinda critical since as afaik it will prevent casting from a deserialized Base to Child and will always require the deserializer to be aware of the child class. I can deal with this from my end (nothing is really preventing me from changing the extra property after subclassing...), but I still think this will be a deal breaker down the road if you have an application that relies on using the core class without knowledge of the subclasses. If other people dont think this is critical and would rather go forward with this pattern lets do it, but in the aind-data-schema we went with the allow extra in the generic class since we wanted to preserve the key:value pairs of the subclass in the base class.

[bruno-f-cruz]

@bruno-f-cruz how about this?

Somehow it fails my test as well. I have tried this solution in the past too, and we ended up converging on the Generics as it seems to be able to preserve the data as a dictionary if deserializing with the base class. Anyway, if others agree that extra parameters should not be allowed and I am worried about this pattern for no good reason, feel free to forget about this test. My feeling is that lack of validation is not a big deal since the validation can always be enforced by the inherited class. The base should simply be able to validate the fields that are required to the Task and, from my point of view, keep information in the form a a generic dict structure of all the fields of the child class in case people want to down cast it later.

[jwong-nd]

@bruno-f-cruz, I have changed the TaskParameters class to a generic as before, but the output of task_creation.py appears to be identical as commit 'suggested change' -- am I missing something here? It seems like both implementations cannot deserialize any subclass information.

[bruno-f-cruz]

@bruno-f-cruz, I have changed the TaskParameters class to a generic as before, but the output of task_creation.py appears to be identical as commit 'suggested change' -- am I missing something here? It seems like both implementations cannot deserialize any subclass information.

This one passes my test:

from aind_behavior_curriculum.behavior import (
    Task, TaskParameters)
from pydantic import Field
from typing import Literal, ClassVar, get_args

class Foo(TaskParameters):
    param1: int = Field(default=1, description="This is a property")
    prop1: int = Field(default=1, description="This is a property")
    prop2: str = Field(default="a", description="This is another property")

_version = Literal["0.1.0"]

class MyTask(Task):
    version: _version = get_args(_version)[0]
    task_parameters: Foo = Field(
        default=Foo(), description="Task parameters.", validate_default=True
    )

# Notice the modifiable tag when modelling as a json-schema
# The tag can be leverage by the specific application to allow/disallow modification
# while remaining "on-curriculum"
print(MyTask.model_json_schema())

instance = MyTask(name="Task", task_parameters=Foo(prop1=2))
print(instance)
# name='Task' description='' version='0.1.0' task_parameters=Foo(param1=1, prop1=2, prop2='a')

instance_json = instance.model_dump_json()
print(instance_json)
# {"name":"Task","description":"","version":"0.1.0","task_parameters":{"param1":1,"prop1":2,"prop2":"a"}}

instance_parent = Task.model_validate_json(instance_json)
print(instance_parent)
# name='Task' description='' version='0.1.0' task_parameters=TaskParameters(param1=1, prop1=2, prop2='a')

instance_prime = MyTask.model_validate_json(instance_parent.model_dump_json())
print(instance_prime == instance)
# True

[bruno-f-cruz]

• Removes Modifiable Attr Tag as this causes an error in serialization. This information is not lost, as all Literals are marked as ‘const’ in Model.model_json_schema(). All fields not marked as Literal/const may be implied as modifiable..

Can you give me an example of how you tested this? I am trying to reproduce and i can deserialize the classes just fine. I am curious to know the failure mode! thanks

[jwong-nd]

This update:

• Adds roundtrip test Bruno mentions in this thread with passing result

As for ModifiableAttr, I found adding this to TaskParameters like this throws an error when Pydantic tries to construct it:

from enum import Enum
from functools import partial
import aind_behavior_curriculum as abc

class AllowModification(str, Enum):
    TRUE = True
    FALSE = False

    def __str__(self):
        return str(self.value)

    def __repr__(self):
        return str(self.value)

ModifiableAttr = partial(Field, allow_modification=AllowModification.TRUE)
class ExampleTaskParameters(abc.TaskParameters):
    field_1: ModifiableAttr = Field(default=0, ge=0.0)

class ExampleTask(abc.Task):
    name: Literal["TaskName"] = "TaskName"
    description: str = Field(default="Example description of task")
    version: abc.SemVerAnnotation = abc.__version__

    task_parameters: ExampleTaskParameters = Field(
        ..., description=ExampleTaskParameters.__doc__
    )

ex_parameters = ExampleTaskParameters()
ex_task = ExampleTask(task_parameters=ex_parameters) 

In any case, the group is okay with sticking with Literals as mentioned in Thursday's meeting

[bruno-f-cruz]

This update:

• Adds roundtrip test Bruno mentions in this thread with passing result

As for ModifiableAttr, I found adding this to TaskParameters like this throws an error when Pydantic tries to construct it:

from enum import Enum
from functools import partial
import aind_behavior_curriculum as abc

class AllowModification(str, Enum):
    TRUE = True
    FALSE = False

    def __str__(self):
        return str(self.value)

    def __repr__(self):
        return str(self.value)

ModifiableAttr = partial(Field, allow_modification=AllowModification.TRUE)
class ExampleTaskParameters(abc.TaskParameters):
    field_1: ModifiableAttr = Field(default=0, ge=0.0)

class ExampleTask(abc.Task):
    name: Literal["TaskName"] = "TaskName"
    description: str = Field(default="Example description of task")
    version: abc.SemVerAnnotation = abc.__version__

    task_parameters: ExampleTaskParameters = Field(
        ..., description=ExampleTaskParameters.__doc__
    )

ex_parameters = ExampleTaskParameters()
ex_task = ExampleTask(task_parameters=ex_parameters) 

In any case, the group is okay with sticking with Literals as mentioned in Thursday's meeting

The attribute is a partial on the Field function. It should be used as any other attribute modifier (e.g. PrivateAttr (https://docs.pydantic.dev/latest/concepts/models/#private-model-attributes) or Field, thus retaining the original flavor of the pydantic syntax). Since you are using it to type your variable it will crash.

I am ok with going with Literal, just keep in mind that I anticipate that fixed values will be the norm, not the other way around, so we are expecting most variables to have the Literal tag, correct?

[jwong-nd]

Okay gotcha! That's how you use ModifiableAttr. If most values are fixed then it makes sense to use this ModifiableAttr for few fields over defining most things Literal. In that case, @hanhou, can you do something like this for the few ModifiableFields in your foraging example in #12 ?

class ExampleTaskParameters(abc.TaskParameters):
    field_1: int = abc.ModifiableAttr(default=0, ge=0.0)

[bruno-f-cruz]

Okay gotcha! That's how you use ModifiableAttr. If most values are fixed then it makes sense to use this ModifiableAttr for few fields over defining most things Literal. In that case, @hanhou, can you do something like this for the few ModifiableFields in your foraging example in #12 ?

class ExampleTaskParameters(abc.TaskParameters):
    field_1: int = abc.ModifiableAttr(default=0, ge=0.0)

Just to clarify if people want to go with Literal by all means. I was just obsessing a bit why you could not get it to work since it seemed to be working with me haha. Thanks for taking a look !

[jwong-nd]

This update:

• Addresses minor comments
• Adds configuration for whitespace stripping