[BUG] Do not use TypeVar to denote the extension of a Flytefile

[eapolinario]

Describe the bug

Linters like pylint are not happy with the blessed way of defining FlyteFile extensions today. A user reported seeing this error when pylint in the sagemaker example:

************* Module example
example.py:1:0: C0114: Missing module docstring (missing-module-docstring)
Exception on node <AssignName._ l.3 at 0x7fe8c7586580> in file '/directory/example.py'
Traceback (most recent call last):
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/inference_tip.py", line 37, in _inference_tip_cached
    result = _cache[func, node]
KeyError: (<function infer_typing_typevar_or_newtype at 0x7fe8c6442670>, <Call l.3 at 0x7fe8c7586640>)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 168, in _data_build
    node, parser_module = _parse_string(data, type_comments=True)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 454, in _parse_string
    parsed = parser_module.parse(data + "\n", type_comments=type_comments)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/_ast.py", line 49, in parse
    return parse_func(string)
  File "/usr/local/lib/python3.8/ast.py", line 47, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 10
    class tar.gz(metaclass=Meta):
             ^
SyntaxError: invalid syntax

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/pylint/checkers/utils.py", line 1258, in safe_infer
    value = next(infer_gen)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/nodes/node_ng.py", line 158, in infer
    results = list(self._explicit_inference(self, context, **kwargs))
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/inference_tip.py", line 44, in _inference_tip_cached
    result = _cache[func, node] = list(func(*args, **kwargs))
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/brain/brain_typing.py", line 127, in infer_typing_typevar_or_newtype
    node = extract_node(TYPING_TYPE_TEMPLATE.format(typename))
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 423, in extract_node
    tree = parse(code, module_name=module_name)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 281, in parse
    return builder.string_build(code, modname=module_name, path=path)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 138, in string_build
    module, builder = self._data_build(data, modname, path)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/astroid/builder.py", line 170, in _data_build
    raise AstroidSyntaxError(
astroid.exceptions.AstroidSyntaxError: Parsing Python code failed:
invalid syntax (<unknown>, line 10)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/pylint/utils/ast_walker.py", line 90, in walk
    callback(astroid)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/pylint/checkers/base/name_checker/checker.py", line 411, in visit_assignname
    inferred_assign_type = utils.safe_infer(assign_type.value)
  File "/usr/local/venvs/test-venv/lib/python3.8/site-packages/pylint/checkers/utils.py", line 1262, in safe_infer
    raise AstroidError from e
astroid.exceptions.AstroidError
example.py:1:0: F0002: example.py: Fatal error while checking 'example.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/vscode/.cache/pylint/pylint-crash-2022-09-12-15.txt'. (astroid-error)

We use this pattern in other places as well, e.g. https://github.com/flyteorg/flytekit/blob/8528268a29a07fe7e9ce9f7f08fea68c41b6a60b/tests/flytekit/unit/core/test_realworld_examples.py#L47 and also in flytesnacks.

Expected behavior

pylint should run without warnings.

Additional context to reproduce

1. at the root of flytekit
2. (Assuming pylint is installed) run pylint plugins/flytekit-aws-sagemaker/flytekitplugins/awssagemaker/training.py

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[eapolinario]

We should invest in using Annotated to define the extension of FlyteFile objects.

[alexdt1982]

Hey, I'm looking to make my first open source contribution. Can I work on this?

[aniketh-varma]

Hi @samhita-alla, can I work on this issue?

#self-assign

[samhita-alla]

@alexdt1982 / @aniketh-varma, please feel free to create a PR. It's first-come, first-serve.

[jerempy]

@samhita-alla I opened a PR for this: flyteorg/flytekit#1236
Could you assign the issue to me?

[hajapy]

What is the origin of the typing.TypeVar('csv') mechanism? As far as I can tell tracing through https://github.com/flyteorg/flytekit/blob/master/flytekit/types/file/file.py#L151-L167, both FlyteFile['csv'] and FlyteFile[typing.TypeVar('csv')] are essentially normalized to the same thing.

FlyteFile['csv'].extension()
'csv'

FlyteFile[typing.TypeVar('csv')].extension()
'csv'

FlyteFile['tar.gz'].extension()
'tar.gz'

FlyteFile[typing.TypeVar('tar.gz')].extension()
'tar.gz'

Would it be appropriate to simply switch using the simple str form and deprecate the typing.TypeVar mechanism to denote the extension?

[samhita-alla]

@eapolinario / @pingsutw ^^^

[eapolinario]

The reason why we use typing.TypeVar as a parameter to FlyteFile is to satisfy python's static type checkers. For example, if you have:

import typing

T = typing.TypeVar('T')

class A(typing.Generic[T]):
    ...

A_int = A[int]
A_int_stringified = A["int"]
A_random_string = A["random_string"]

reveal_type(A)
reveal_type(A_int)
reveal_type(A_int_stringified)
reveal_type(A_random_string)

here's what mypy has to say about this code:

❯ mypy example.py
example.py:10: error: Name "random_string" is not defined
...

Notice how we can't simpy use a string as a type variable.

That's why my initial suggestion to solve this problem was to use typing.Annotated as @samhita-alla suggested in the PR.