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

Updates for Python 3.11 compatibility #113

Merged
merged 3 commits into from
Sep 24, 2023
Merged

Updates for Python 3.11 compatibility #113

merged 3 commits into from
Sep 24, 2023

Conversation

Silarn
Copy link
Member

@Silarn Silarn commented Sep 16, 2023

Some of this may also just be general cleanup for recent changes

@Holt59
Copy link
Member

Holt59 commented Sep 19, 2023

Is this still needed?

@Silarn
Copy link
Member Author

Silarn commented Sep 19, 2023

I think so, at least this all seems to be fixed for updates to pyqt6 and the differences in the pyi files as well as the necessary dependency version update for 3.11.

Copy link
Member

@Holt59 Holt59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me except for the Optional stuff that should use the new | None syntax.

@@ -33,7 +33,7 @@ def allFiles(self) -> list[str]:


class BasicGameSaveGameInfoWidget(mobase.ISaveGameInfoWidget):
def __init__(self, parent: QWidget, get_preview: Callable[[Path], Path | None]):
def __init__(self, parent: QWidget, get_preview: Optional[Callable[[Path], Optional[QPixmap | Path | str]]]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional should not be used anymore, you can use | None when needed. I don't think making get_preview optional is needed, simply default it to a lambda p: None. I thought this was already done in an existing PR 🤔

@@ -89,9 +89,9 @@ def setSave(self, save: mobase.ISaveGame):


class BasicGameSaveGameInfo(mobase.SaveGameInfo):
def __init__(self, get_preview: Callable[[Path], Path | None] | None = None):
def __init__(self, get_preview: Optional[Callable[[Path], Optional[QPixmap | Path | str]]] = None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above for Optional.

super().__init__()
self._get_preview = get_preview
self._get_preview: Optional[Callable[[Path], Optional[QPixmap | Path | str]]] = get_preview
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to type here since get_preview (the parameter) is typed.

@Holt59
Copy link
Member

Holt59 commented Sep 19, 2023

Also, CI will probably fail for | None because the Python version needs to be updated to 3.11 in the workflow file.

@Silarn
Copy link
Member Author

Silarn commented Sep 19, 2023

I made the changes because my IDE was complaining that None was not supposed to be valid. I'll have to do this in a few hours.

@Holt59
Copy link
Member

Holt59 commented Sep 19, 2023

I made the changes because my IDE was complaining that None was not supposed to be valid. I'll have to do this in a few hours.

I don't know what IDE you're using, but it's probably using a Python version lower than 3.10.

@Holt59 Holt59 merged commit adcd06d into master Sep 24, 2023
2 checks passed
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 this pull request may close these issues.

2 participants