Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Python 3.12 #1384

Closed
4 tasks done
tbennun opened this issue Oct 6, 2023 · 9 comments · Fixed by #1386
Closed
4 tasks done

Support Python 3.12 #1384

tbennun opened this issue Oct 6, 2023 · 9 comments · Fixed by #1386
Assignees

Comments

@tbennun
Copy link
Collaborator

tbennun commented Oct 6, 2023

To add support for Python 3.12:

  • What's new in Python 3.12? Are there any modified AST features or any added syntactic features?
  • Does Python 3.12 work with the current test suite (change github workflows)?
  • Modify setup.py as necessary to add support in the next release
  • Release a major version (v0.15)
@tbennun
Copy link
Collaborator Author

tbennun commented Oct 6, 2023

@BenWeber42 @phschaad could we add this to the next release, which should be a major release?

@phschaad
Copy link
Collaborator

phschaad commented Oct 6, 2023

Sure, we'll make sure to include it, thanks

@alexnick83 alexnick83 linked a pull request Oct 6, 2023 that will close this issue
@tbennun
Copy link
Collaborator Author

tbennun commented Oct 6, 2023

I'm answering my own comments in the top message:

  • Nothing new that would affect AST parsing in 3.12 (new f-strings should be supported as-is)
  • Testing is in progress
  • Done in Python 3.12 #1386
  • Someone needs to release a version if point (2) works

@alexnick83
Copy link
Contributor

  • Nothing new that would affect AST parsing in 3.12 (new f-strings should be supported as-is)

There is a new type statement, which I suppose should be added to the disallowed statements (it is used to define types; I don't think there is currently any use in a DaCeProgram). I also see a lot of deprecation warnings (to be enforced in Python 3.14), so we may as well prepare early. Working on it ...

@tbennun
Copy link
Collaborator Author

tbennun commented Oct 7, 2023

Instead of disallowing them we might want to skip those as they’re noops.

@alexnick83
Copy link
Contributor

After doing some testing, I decided to disallow the type statement (ast.TypeAlias). My justification is the following:

Consider the following DaCe program using the new type statement:

    @dace.program
    def type_statement():
        type Scalar[T] = T
        A: Scalar[dace.float32] = 0
        return A

Without any changes to the frontend, the above program will fail because Scalar has not been defined. If I make a visitor method for ast.TypeAlias that returns None, there will be a warning about the type annotation but the program will run. However, the output will be [0], i.e. an integer. Compare this to the following DaCe program, which will correctly return [0.], i.e., a floating point number:

    @dace.program
    def type_statement():
        A: dace.float32 = 0
        return A

Therefore, I believe that considering the type statement a no-op will only result in "minor" errors for users that use the type statement, which will be difficult to debug. I think the best course of action is to explicitly disallow the statement until we are ready to properly support it.

@BenWeber42
Copy link
Contributor

I'm also in favor of disallowing for now. Searching ast module for 3.12, there are more changes. New classes:

  • ast.TypeAlias
  • ast.TypeVar
  • ast.ParamSpec
  • ast.TypeVarTuple

And some classes got a type_params attribute (ast.FunctionDef, ast.ClassDef, ast.AsyncFunctionDef).
The new typing features seem quite nice (https://peps.python.org/pep-0695/), but I'm not sure what it means for how we've handled typing in DaCe's python front-end so far.

@tbennun
Copy link
Collaborator Author

tbennun commented Oct 9, 2023

On second thought, I agree with you both.

@phschaad
Copy link
Collaborator

phschaad commented Oct 9, 2023

I will make sure to handle the release with @BenWeber42 before the next dace meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants