Skip to content

=Avoid converting ObservableCollection<T> into python list #773

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 14, 2018
Merged

=Avoid converting ObservableCollection<T> into python list #773

merged 3 commits into from
Nov 14, 2018

Conversation

fdv1
Copy link
Contributor

@fdv1 fdv1 commented Nov 11, 2018

Avoid converting objects implementing INotifyPropertyChanged (eg:ObservableCollection) into python list. This would fix #770

@den-run-ai
Copy link
Contributor

i think as a temp fix this is ok, but we are planning to revert this list autoconversion completely once a viable alternative is decided.

@den-run-ai
Copy link
Contributor

here is the original issue:

#514

@codecov
Copy link

codecov bot commented Nov 12, 2018 •
edited
Loading

Codecov Report

Merging #773 into master will increase coverage by 8.3%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #773     +/-   ##
=========================================
+ Coverage   68.55%   76.86%   +8.3%     
=========================================
  Files           1       62     +61     
  Lines         283     5752   +5469     
  Branches        0      909    +909     
=========================================
+ Hits          194     4421   +4227     
- Misses         89     1029    +940     
- Partials        0      302    +302
Flag Coverage Δ
#setup_linux 68.55% <ø> (ø) ⬆️
#setup_windows 75.9% <0%> (?)
Impacted Files Coverage Δ
src/runtime/converter.cs 81.56% <0%> (ø)
src/runtime/pyansistring.cs 100% <0%> (ø)
src/runtime/constructorbinder.cs 54.54% <0%> (ø)
src/runtime/methodbinder.cs 89.8% <0%> (ø)
src/runtime/Util.cs 37.5% <0%> (ø)
src/runtime/delegatemanager.cs 88.18% <0%> (ø)
src/runtime/eventbinding.cs 46.51% <0%> (ø)
src/runtime/pyiter.cs 77.27% <0%> (ø)
src/runtime/pynumber.cs 100% <0%> (ø)
src/runtime/interop27.cs 100% <0%> (ø)
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88d61a9...a3a3ef5. Read the comment docs.

@fdv1
Copy link
Contributor Author

fdv1 commented Nov 12, 2018

Great! sorry for the mess with the featured branch...

@fdv1 fdv1 closed this Nov 12, 2018
@filmor filmor reopened this Nov 14, 2018
@filmor
Copy link
Member

filmor commented Nov 14, 2018

@fdv1 I guess you accidentally closed this one, is this correct?

@filmor filmor merged commit f807a81 into pythonnet:master Nov 14, 2018
@jnsquire
Copy link

This is still broken for some use cases. See, for example:

https://github.com/icsharpcode/SharpDevelop/blob/master/src/AddIns/DisplayBindings/AvalonEdit.AddIn/Src/IconBarManager.cs

Where Bookmarks is returned as an IList and thus loses the ability to mutate the underlying ObservableCollection.

@maresb maresb mentioned this pull request Jun 26, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@den-run-ai den-run-ai den-run-ai approved these changes

@vmuriart vmuriart Awaiting requested review from vmuriart

@filmor filmor Awaiting requested review from filmor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pythonnet converting ObservableCollection into python list
4 participants
@fdv1 @den-run-ai @filmor @jnsquire