Visitar URL original
`Min`/`MaxSupportedVersion` and `IsSupportedVersion` on `PythonEngine` by lostmsu · Pull Request #1798 · pythonnet/pythonnet · GitHub
Skip to content

Min/MaxSupportedVersion and IsSupportedVersion on PythonEngine - #1798

Merged
filmor merged 1 commit into
pythonnet:masterfrom
losttech:SupportedVersions
May 26, 2022
Merged

Min/MaxSupportedVersion and IsSupportedVersion on PythonEngine#1798
filmor merged 1 commit into
pythonnet:masterfrom
losttech:SupportedVersions

Conversation

@lostmsu

@lostmsu lostmsu commented May 25, 2022

Copy link
Copy Markdown
Member

Does this close any currently open issues?

fixes #1724

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • Updated the CHANGELOG

@lostmsu
lostmsu force-pushed the SupportedVersions branch from 52a00c0 to 1492d7d Compare May 25, 2022 21:39
@lostmsu

lostmsu commented May 25, 2022

Copy link
Copy Markdown
Member Author

:/ I am puzzled. What does this change have to do with segfaults? Only on *nix? Ugh...

filmor
filmor reviewed May 25, 2022
View reviewed changes
Comment thread src/runtime/PythonEngine.cs
Comment thread src/runtime/PythonEngine.cs
}

public static Version MinSupportedVersion => new(3, 7);
public static Version MaxSupportedVersion => new(3, 10, int.MaxValue, int.MaxValue);

@filmor filmor May 25, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe something more "readable", like 999 instead of maxint?

@lostmsu lostmsu May 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought of that, but it won't work if there are builds of Python that use build number not as patch (e.g. patch .4), but in a Windows manner like .19041.365, or maybe do MaxSupportedVersionExclusive. Alternatively, we can only leave the IsSupportedVersion check and provide an error string in a separate property.

What do you think?

@filmor

filmor commented May 26, 2022

Copy link
Copy Markdown
Member

:/ I am puzzled. What does this change have to do with segfaults? Only on *nix? Ugh...

Just tested it as well, it only happens on Mono...

@filmor

filmor commented May 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Further observations:

  • The culprit is the call, the comparisons in the tests seem fine
  • Only the enum conversions with max int/long/ulong fail, if I remove these, the tests run though
  • Only happens when all tests are run in order

@filmor filmor mentioned this pull request May 26, 2022
Merged
5 tasks
@filmor
filmor merged commit 1492d7d into pythonnet:master May 26, 2022
@lostmsu
lostmsu deleted the SupportedVersions branch May 26, 2022 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@filmor filmor filmor left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lostmsu @filmor