Improve ListArtifact

[collindutter]

• I have read and agree to the contributing guidelines for submitting new pull requests.

Describe your changes

Added

• Generic type support to ListArtifact.
• Iteration support to ListArtifact.

Issue ticket number and link

NA

[codecov]

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
griptape/tasks/tool_task.py	50.00%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[collindutter]

I thought the type check was dependency related but there's something else going on. Looking into it.

[vachillo]

is validate_uniform_types still applicable here?
what would happen with:

ListArtifact[BlobArtifact]([TextArtifact("foo")], validate_uniform_types=False)

?

[collindutter]

I think validate_uniform_types is still useful as a runtime check (although we don't actually use it anywhere lol). In your example, it would fail type check but succeed at runtime.

[dylanholmes]

Awesome!

[dylanholmes]

I think this should set covariant=True.

This will allow for example, passing ListArtifact[TextArtifact] where a ListArtifact[BaseTextArtifact] is required.

Relating it to the definition of a covariant type, if ListArtifact[T] is covariant in T if ListArtifact[S] is considered a subtype of ListArtifact[T] whenever S is a subtype of T. Kind of like the bounds=, but where the value of bounds is determined by a particular usage rather than all usages.

[collindutter]

Great suggestion! Will update.

[dylanholmes]

Could you remind me why value is Sequence instead of list? If you wanted ListArtifact to subclass a collection, would you pick Sequence or list? (I expect the choice to be based on whether or not ListArtifact should be mutable or not)

We currently implement the interface of at least Sequence (required methods are __getitem__, __len__, __contains__), but we just don't have the base class.

[collindutter]

list is not covariant, Sequence is. Here is an example pyright error:

    Return type mismatch: base method returns type "BaseArtifact | list[BaseArtifact]", override returns type "list[CsvRowArtifact]"
      Type "list[CsvRowArtifact]" is incompatible with type "BaseArtifact | list[BaseArtifact]"
        "list[CsvRowArtifact]" is incompatible with "BaseArtifact"
        "list[CsvRowArtifact]" is incompatible with "list[BaseArtifact]"
          Type parameter "_T@list" is invariant, but "CsvRowArtifact" is not the same as "BaseArtifact"
          Consider switching from "list" to "Sequence" which is covariant (reportIncompatibleMethodOverride)

[dylanholmes]

TIL!

After looking into why list is not covariant and why Sequence is, its because list is mutable!

example:

class Animal:
    pass

class Dog(Animal):
    pass

dogs: list[Dog] = [Dog(), Dog()]
animals: list[Animal] = dogs  # This would be allowed if lists were covariant

animals.append(Animal())  # Adding an Animal (not necessarily a Dog!) to a list of Dogs

So this leads me to the question, ListArtifact is not intended to be mutable, is it?

[collindutter]

ListArtifact is not intended to be mutable, is it?

Good question, probably not. I can investigate changing back to list, but it will have implications throughout the codebase so I think it should be done in a separate PR.

[collindutter]

Many Artifact refactors incoming