FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException by tminka · Pull Request #1326 · pythonnet/pythonnet · GitHub

/ pythonnet Public

Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException - #1326

Merged
lostmsu merged 4 commits into
pythonnet:masterfrom
tminka:generics-exception
Dec 18, 2020
ConversationCommits (4)ChecksFiles changed
Merged

Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException#1326
lostmsu merged 4 commits into
pythonnet:masterfrom
tminka:generics-exception

Conversation

tminka commented Dec 17, 2020

Copy link
Copy Markdown
Contributor

What does this implement/fix? Explain your changes.

Fixes #1325

Does this close any currently open issues?

Fixes #1325

Any other comments?

...

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

dnfadmin commented Dec 17, 2020 •
edited
Loading

Copy link
Copy Markdown


All CLA requirements met.

tminka commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

The tests appear to be failing for the reasons addressed by PR #1260. Applying those changes should make the tests pass.

lostmsu commented Dec 17, 2020

Copy link
Copy Markdown
Member

@tminka I am a bit lost. In the comment to that PR you mentioned, that de7e1ac solves the issue. But the PR itself does not contain that commit.

filmor requested changes Dec 17, 2020
View reviewed changes

filmor left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Very nice addition, thank you. If you can just move the test to Python, this is good to go :)

Comment thread src/embed_tests/TestList.cs Outdated
Comment thread src/runtime/genericutil.cs Outdated

tminka commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

@lostmsu My only point was that the test failures are the same as the ones in the current master, and those are due to finalization/deallocation bugs.

tminka force-pushed the generics-exception branch 2 times, most recently from f2ceaf6 to 24683d0 Compare December 18, 2020 16:25

filmor commented Dec 18, 2020

Copy link
Copy Markdown
Member

Please make this just a normal Python test. You can run pytest from within VisualStudio using our normal APIs (import pytest; pytest.main()) if you want.

filmor commented Dec 18, 2020

Copy link
Copy Markdown
Member

Ah, I see that you have already moved the test. Would you be okay with splitting the EmbeddedPythonTest into a separate PR? I think this does have merit, but it's independent of the actual fix here.

tminka force-pushed the generics-exception branch from 5db975a to d1103ec Compare December 18, 2020 20:49
Incorrectly using a non-generic type with type parameters now produce…
0fe042c 
…s a helpful Python error instead of throwing NullReferenceException
Inlined GenericUtil.GenericsByName into GenericByName.
8e754b6 
Removed unused GenericUtil.GenericsForType.
Other code quality improvements.
Moved the test to Python.
612c0cc 
Added EmbeddedPythonTest with the capability to run any desired Python tests within NUnit.
Removed EmbeddedPythonTest
20e673f
tminka force-pushed the generics-exception branch from d1103ec to 20e673f Compare December 18, 2020 21:05
lostmsu merged commit d9e15a7 into pythonnet:master Dec 18, 2020
tminka deleted the generics-exception branch December 21, 2020 00:07
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lostmsu lostmsu left review comments
filmor filmor requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

NullReferenceException when non-generic type is given type parameters

4 participants

Back | FazBrowse Home | New Git URL