search

patrick-kidger / torchtyping

Type annotations and dynamic checking for a tensor's shape, dtype, names, etc.
Apache License 2.0
1.41k stars 34 forks source link

fixes issue #15: allow named sizes for named dimensions #17

Closed teichert closed 3 years ago

teichert commented 3 years ago

fixes #15, tests, and updates README

teichert commented 3 years ago

Thanks for looking through this! I've responded to your comments and also added the tests for None: str and other: str.

Here is a test that probably should be added (it would currently fail):

def test_str_nonidentifier_should_fail():
    with pytest.raises(TypeError):

        def func(x: TensorType["x":"y+2"]):
            pass

The point is that we should probably only allow str for a dimension size if it is a legit identifier (alphanumeric or underscore but doesn't start with a number). Do you want me to add the test and then make the fix? I can't think of a good reason not to.

patrick-kidger commented 3 years ago

Thanks for the changes.

I actually wouldn't worry about that last one. For the time being any string is a valid way of referring to a dimension, and this then becomes an exception to that rule. And once #2 happens, I don't see a reason to avoid functions in the other named dimensoin.

teichert commented 3 years ago

Okay; sounds good! Let me know if you need anything else from me.

patrick-kidger commented 3 years ago

LGTM! Thanks for contributing.