Skip to content

Explorarion of the new *Reference types: reimplemented some of the PyList members using BorrowedReference #1068

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

Merged
merged 2 commits into from
Mar 3, 2020

Conversation

lostmsu
Copy link
Member

@lostmsu lostmsu commented Feb 27, 2020

Just playing with the new *Reference types to see what patterns might arise.

This adds:

  • PyObject.Reference internal property, that exposes the same pointer as obj, but in a way, that it is clear, that the reference is borrowed
  • PyObject(BorrowedReference) constructor, that extracts the address from the reference and IncRefs it before storing in the instance
  • a few of the PyList_* methods now explicitly take BorrowedReference instead of raw pointers

@lostmsu
Copy link
Member Author

lostmsu commented Feb 29, 2020

@amos402 I noticed you had to make code changes using PyList in #958 . If you review this, those changes might not be needed anymore.

@lostmsu lostmsu added this to the 2.4.1 milestone Mar 3, 2020
@filmor filmor merged commit 9fd877e into pythonnet:master Mar 3, 2020
@amos402
Copy link
Member

amos402 commented Mar 8, 2020

@lostmsu Still needs, because I need to put them into collection and return somewhere. That's why I suggest making them copyable before.

@lostmsu lostmsu deleted the PR/ReferenceExploration branch March 9, 2020 19:30
@amos402 amos402 mentioned this pull request Mar 10, 2020
4 tasks
@lostmsu lostmsu modified the milestones: 2.4.1, 2.5.0 Apr 23, 2020
AlexCatarino pushed a commit to QuantConnect/pythonnet that referenced this pull request Jun 29, 2020
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.

3 participants