fix faulty comparisons

[XzzX]

fix

pyiron_workflow/mixin/semantics.py:100: error: Non-overlapping container check (element type: "Semantic[ParentType]", container item type: "str")  [comparison-overlap]
pyiron_workflow/storage.py:316: error: Non-overlapping equality check (left operand type: "type[PickleStorage]", right operand type: "StorageInterface")  [comparison-overlap]

and enable corresponding mypy check.

[github-actions]

👈 Launch a binder notebook on branch pyiron/pyiron_workflow/fix_comparisons

[XzzX]

Since the unit test fails:
What was this line supposed to do? or (isinstance(backend, StorageInterface) and value != backend)?

[XzzX]

Regarding the other error: passing True to autoload is a type violation autoload: Literal["pickle"] | StorageInterface | None = None,. Since mypy is not enabled for the tests this is not caught.

[liamhuber]

Regarding the other error: passing True to autoload is a type violation autoload: Literal["pickle"] | StorageInterface | None = None,. Since mypy is not enabled for the tests this is not caught.

Yes, this agreed, this is just a straight-up bug. I guess the final yield in available_backends managed to return the first standard backend in this case so the error didn't show up.

Since the unit test fails: What was this line supposed to do? or (isinstance(backend, StorageInterface) and value != backend)?

Good question. Honestly, I don't know what I was thinking here. I'm working through a patching test now though. Your PR looks good to me, but despite that something is still off -- the failing test is correct and I really do expect to find two instances in the return to available_backends.

I'm stacking on a patch for the tests now. I fixed the wrong datatype and will try to keep your changes while still getting both the expected backends for the failing storage test.

[liamhuber]

Your PR looks good to me, but despite that something is still off -- the failing test is correct and I really do expect to find two instances in the return to available_backends.

Ah, I see it now. You terminate with an isisntance check, and the second back end I've provided has the same type so we're not getting it. I just need to think whether a class based check is really fair, or if the backends might be able to have different configurations such that in some cases we genuinely want to keep two backend instances of the same class.

[liamhuber]

At any rate, that explains why I was doing a direct value check.

And yes, StorageInterface doesn't constrain __init__ or make any promises about the interface state, and already the one concrete case we have,PickleStorage, lets us configure whether or not we fall back to cloudpickle. So I think we genuinely want the value check here.

[liamhuber]

Yes, I like this direction. What's here actually changes the behaviour in storage.available_backends a little bit though, and the test failure was legitimate vis a vis the intention of the function. I stacked on a patch in #580, so let me know what you think there, after which I'm happy here.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.02% (target: -1.00%)	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (1ca0dfb)	3429	3133	91.37%
Head commit (2373aa8)	3423 (-6)	3127 (-6)	91.35% (-0.02%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#579)	5	5	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

[liamhuber]

All checks are passing