phi-friday commented on code in PR #40208:
URL: https://github.com/apache/airflow/pull/40208#discussion_r1637835611
##########
airflow/utils/python_virtualenv_script.jinja2:
##########
@@ -16,7 +16,9 @@
specific language governing permissions and limitations
under the License.
-#}
-
+{% if import_annotations | default(False) %}
+from __future__ import annotations
Review Comment:
I wasn't sure about the side effects, so this is how I handled it.
I don't know if this will actually happen, but it could cause an error.
```python
def annotation_error(value: int = 1) -> None:
import inspect
sig = inspect.signature(annotation_error)
maybe_int = sig.parameters["value"].annotation
print(repr(maybe_int))
some_int = maybe_int(1)
print(some_int)
def with_import_annotaions() -> None:
import inspect
import subprocess
import sys
func = inspect.getsource(annotation_error)
py_source = f"""
from __future__ import annotations
{func}
if __name__ == "__main__":
assert callable(annotation_error)
annotation_error()
"""
# error
subprocess.run([sys.executable, "-c", py_source], check=True)
if __name__ == '__main__':
annotation_error()
# output:
# <class 'int'>
# 1
with_import_annotaions()
# output:
# 'int'
# TypeError: 'str' object is not callable
```
Of course, it's much easier not to consider these cases.
It's also unlikely that this is the case.
(However, I've seen similar issues with `pydantic`).
Considering that the implementation is much simpler and will be the default
in the next Python version, I think it's better to change it as you said.
I'll make it as soon as possible, and fix the test code that caused the
error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
