8000 implement list codec by koubaa · Pull Request #1084 · pythonnet/pythonnet · GitHub
[go: up one dir, main page]

Skip to content

implement list codec #1084

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 25 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
respond to PR comments
  • Loading branch information
koubaa committed Feb 16, 2021
commit c43446e7f339c9cd6eaf82549b13f1a9e15b0757
20 changes: 18 additions & 2 deletions src/runtime/CollectionWrappers/IterableWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,28 @@ namespace Python.Runtime.CollectionWrappers
{
internal class IterableWrapper<T> : IEnumerable<T>
{
protected PyObject iterObject;
protected PyObject pyObject;

public IterableWrapper(PyObject pyObj)
{
pyObject = pyObj;
iterObject = new PyObject(Runtime.PyObject_GetIter(pyObj.Handle));
}

private void propagateIterationException()
{
var err = Runtime.PyErr_Occurred();
if (err != null && err != Exceptions.StopIteration)
{
Runtime.CheckExceptionOccurred();
}
}

IEnumerator IEnumerable.GetEnumerator()
{
if (pyObject == null) yield break;
PyObject iterObject = new PyObject(Runtime.PyObject_GetIter(pyObject.Handle));
IntPtr item;

while ((item = Runtime.PyIter_Next(iterObject.Handle)) != IntPtr.Zero)
{
object obj = null;
Expand All @@ -30,10 +40,14 @@ IEnumerator IEnumerable.GetEnumerator()
Runtime.XDecref(item);
yield return obj;
}

propagateIterationException();
}

public IEnumerator<T> GetEnumerator()
{
if (pyObject == null) yield break;
PyObject iterObject = new PyObject(Runtime.PyObject_GetIter(pyObject.Handle));
IntPtr item;
while ((item = Runtime.PyIter_Next(iterObject.Handle)) != IntPtr.Zero)
{
Expand All @@ -47,6 +61,8 @@ public IEnumerator<T> GetEnumerator()
Runtime.XDecref(item);
yield return (T)obj;
}

propagateIterationException();
}
}
}
17 changes: 7 additions & 10 deletions src/runtime/CollectionWrappers/ListWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,11 @@ public T this[int index]
{
get
{
IntPtr item = Runtime.PySequence_GetItem(pyObject.Handle, index);
object obj;
var item = Runtime.PyList_GetItem(pyObject.Handle, index);
var pyItem = new PyObject(item);
Copy link
Member

Choose a reason for hiding this comment

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

Dispose this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lostmsu again I thought the point of making it IDisposable was that the finalizer would take care of that

Copy link
Member

Choose a reason for hiding this comment

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

Wrap it by using statement is better. If you knew it's no use anymore you should dispose it manually instead of waiting for the auto collection to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lostmsu @amos402 sorry, I might be missing context (I rebased and I don't know if this comment lines up with the code it was pointing to initially).

What specifically should I be disposing or wrap in a using statement?

Copy link
Member

Choose a reason for hiding this comment

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

using (var pyItem = new PyObject(item)) { }

Copy link
Member

Choose a reason for hiding this comment

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

@koubaa this still needs a fix. With C# 8+ which we use now, it should just be using var pyItem = new PyObject(item); so that the Python object, corresponding to the item would be disposed and not pollute finalizer thread unless duplicated.


if (!Converter.ToManaged(item, typeof(T), out obj, true))
{
Runtime.XDecref(item);
if (!Converter.ToManaged(pyItem.Handle, typeof(T), out object obj, true))
Runtime.CheckExceptionOccurred();
}

return (T)obj;
}
Expand All @@ -31,10 +28,10 @@ public T this[int index]
if (pyItem == IntPtr.Zero)
throw new Exception("failed to set item");

var result = Runtime.PySequence_SetItem(pyObject.Handle, index, pyItem);
var result = Runtime.PyList_SetItem(pyObject.Handle, index, pyItem);
Runtime.XDecref(pyItem);
if (result == -1)
throw new Exception("failed to set item");
Runtime.CheckExceptionOccurred();
}
}

Expand All @@ -46,7 +43,7 @@ public int IndexOf(T item)
public void Insert(int index, T item)
{
if (IsReadOnly)
throw new NotImplementedException();
throw new InvalidOperationException("Collection is read-only");

IntPtr pyItem = Converter.ToPython(item, typeof(T));
if (pyItem == IntPtr.Zero)
Expand All @@ -55,7 +52,7 @@ public void Insert(int index, T item)
var result = Runtime.PyList_Insert(pyObject.Reference, index, pyItem);
Runtime.XDecref(pyItem);
if (result == -1)
throw new Exception("failed to insert item");
Runtime.CheckExceptionOccurred();
}

public void RemoveAt(int index)
Expand Down
14 changes: 12 additions & 2 deletions src/runtime/CollectionWrappers/SequenceWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ public int Count
{
get
{
return (int)Runtime.PySequence_Size(pyObject.Handle);
var size = Runtime.PySequence_Size(pyObject.Handle);
if (size == -1)
{
Runtime.CheckExceptionOccurred();
throw new Exception("Unable to get sequence size!");
}

return (int)size;
}
}

Expand All @@ -34,7 +41,10 @@ public void Clear()
throw new NotImplementedException();
var result = Runtime.PySequence_DelSlice(pyObject.Handle, 0, Count);
if (result == -1)
{
Runtime.CheckExceptionOccurred();
throw new Exception("failed to clear sequence");
}
}

public bool Contains(T item)
Expand All @@ -46,7 +56,7 @@ public bool Contains(T item)
return false;
}

protected T getItem(int index)
private T getItem(int index)
{
IntPtr item = Runtime.PySequence_GetItem(pyObject.Handle, index);
object obj;
Expand Down
0