Type-checking columns

[mikapfl]

Describe the bug

This:

import scmdata

def test() -> scmdata.ScmRun:
    columns = {"regions": ["here", "there"], "model": "test"}
    return scmdata.ScmRun(
        index=[1, 2, 3],
        data=np.ones((3, 2)),
        columns=columns,
    )

does not type-check, with the error:

error: Argument "columns" to "ScmRun" has incompatible type "dict[str, Sequence[str]]"; expected "Mapping[Hashable, str | int | float | Iterable[str | int | float]] | None"  [arg-type]

If you specify the columns directly in the call:

def test() -> scmdata.ScmRun:
    return scmdata.ScmRun(
        index=[1, 2, 3],
        data=np.ones((3, 2)),
        columns={"regions": ["here", "there"], "model": "test"},
    )

It type-checks no problem.

Expected behaviour

I expect this common idiom to be fine.

Additional context

I'm confused why this fails. Apparently, Hashable isn't properly supported by mypy? Something about mutability etc.

Maybe we just (in the type hints) put our foot to the ground and say that metadata column names have to be str?

Or maybe there is another solution which I'm missing.

[lewisjared]

Let's use str instead of hashable. It is probably good to enforce string-based metadata columns anyway. What would an integer as a column name represent???

[mikapfl]

What would an integer as a column name represent???

You're not thinking big enough, tuple are also hashable 😉

Let's use str instead of hashable.

To fix this, we also have to use Mappable instead of dict, otherwise I get:

thing.py:7: error: Argument 1 to "a" has incompatible type "Dict[str, str]"; expected "Dict[str, Union[str, Iterable[str]]]"  [arg-type]
thing.py:7: note: "Dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
thing.py:7: note: Consider using "Mapping" instead, which is covariant in the value type
Found 1 error in 1 file (checked 1 source file)

As you said, typing support is not quite as mature in Python as it is in other languages…

[lewisjared]

Updated columns type hint to columns: Mapping[str, MetadataValue | Iterable[MetadataValue]] | None in #262