-
Notifications
You must be signed in to change notification settings - Fork 4
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
compatibility with python on longleaf. To remove ?
- Loading branch information
Showing
2 changed files
with
2 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7e2a061
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally don't comment on commits outside of PRs, but since this was referenced in GH-280 this caught my eye. This is not resolved by upgrading from python 3.10 to 3.11, future annotations are still in place (in fact I don't think it made it into 3.13 either even). However, I'm confused why this is required here? I don't see any circular type hints between these two files which is typically what this is meant to fix. I was able to check out the branch and run the unit tests successfully without the
from __future__ import annotations
lines in either file.7e2a061
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, you are right to comment here, I wanted to bring that to your attention. There are still some python3.9 env
and without this we get as error
7e2a061
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
straightforward fix is to update the environement, but constantly things running at the moment. I think we'll remove that in the PR.
7e2a061
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. This is because of the
str | int
syntax, for python 3.9 need to useUnion[str, int]
. Although that would be quite the change to support 3.9 in the mainstreamgempyor
. Yeah, I think this is the right approach for the moment, hopefully the HPC install script PR makes it easy to correct this soon.7e2a061
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, can't wait to try this after the current rush.