GNOME Bugzilla – Bug 672463
Make tree models set default values when receiving None
Last modified: 2012-03-21 15:03:43 UTC
According to http://mail.gnome.org/archives/python-hackers-list/2012-March/msg00007.html a consequence of fixing https://bugzilla.gnome.org/show_bug.cgi?id=671610 was the appearance of extra warnings when you try to insert NULL values in a tree model, instead of silently setting the default value. The original behavior is convenient and keeps the code simpler when, for example, you are adding a row that is going to be a separator and therefore most of its values are irrelevant. So I was suggested to file this bug.
The current test suite reproduces these warnings.
Created attachment 210233 [details] [review] Fix warnings on None values in added tree/list store rows This restores the behaviour before the "atomic signals" patch, by skipping the None values again. The test suite already reproduces the warnings before the patch, and has coverage for both the atomic/row based append() and friends, as well as changing rows through _set_row(), so no new tests necessary.
Created attachment 210238 [details] [review] Fix warnings on None values in added tree/list store rows Updated patch which makes the test suite actually abort on any warning in the TestGtk class.
Review of attachment 210238 [details] [review]: ::: gi/overrides/Gtk.py @@ +812,3 @@ + columns = [] + cur_col = -1 + for value in row: for cur_col, value in enumerate(row) @@ +823,2 @@ def set_row(self, treeiter, row): + (converted_row, columns) = self._convert_row(row) No need for () @@ +956,3 @@ def _do_insert(self, position, row): if row is not None: + (row, columns) = self._convert_row(row) Ditto ::: tests/test_overrides.py @@ +561,3 @@ class TestGtk(unittest.TestCase): + def setUp(self): I think this way is the wrong way of doing it, as we cannot catch warnings in tests that doesn't happen in TestGtk. Rather it should do; 1) set the fatal log level to warning when importing glib 2) bump it up to error/critial when needed, for tests that's actually testing bad behavior.
Created attachment 210239 [details] [review] Fix warnings on None values in added tree/list store rows Thanks for the review! Updated patch attached which now globally enables fatal warnings. I left a comment about when to temporarily bump it back up, but right now we do not have any warnings in test_overrides.py (but might have some day).
Review of attachment 210239 [details] [review]: Perfect, thanks.
Committed to master, as we now have a pygobject-3-2 branch. I'm inclined to ask for a hard freeze exception for this, to also cherry-pick it into 3-2.