Skip to content
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

[BUG?] unsupported operand type(s) for +: 'ParentNode' and 'int' #828

Closed
2 of 3 tasks
oliver-sanders opened this issue Feb 21, 2024 · 1 comment · Fixed by #829
Closed
2 of 3 tasks

[BUG?] unsupported operand type(s) for +: 'ParentNode' and 'int' #828

oliver-sanders opened this issue Feb 21, 2024 · 1 comment · Fixed by #829

Comments

@oliver-sanders
Copy link

oliver-sanders commented Feb 21, 2024

Description:

My use case is no longer working with urwid 2.6.2/3. The breakage appears to come from this change:

https://github.com/urwid/urwid/pull/824/files#diff-4c88378f9ea5730d04299a47d1ddd0842ae43a8dbaba7bdaf78e3b7762078189R674

Where self._body.get_next was replaced by bottom_pos + 1. In my case bottom_pos is a ParentNode not an integer, resulting in a TypeError.

It looks like the get_next interface is a more generic solution, the TreeWalker.get_next implementation appears to be expecting some form of TreeNode which suggests the integer type might not be a safe assumption:

urwid/urwid/treetools.py

Lines 423 to 429 in 36bf037

# pylint: disable=arguments-renamed # its bad, but we should not change API
def get_next(self, start_from) -> tuple[TreeWidget, TreeNode] | tuple[None, None]:
target = start_from.get_widget().next_inorder()
if target is None:
return None, None
return target, target.get_node()

I don't have a minimal reproducible example at present, I'm not sure if this is somehow resulting from misuse of urwid, hence BUG?.

Affected versions (if applicable)
  • master branch (specify commit)
  • Latest stable version from pypi - 2.6.2 & 2.6.3
  • Other (specify source)
Steps to reproduce (if applicable)

No minimal reproducible example at present.

Expected/actual outcome

No TypeError.

@wardi
Copy link
Collaborator

wardi commented Feb 21, 2024

That's right we shouldn't have code that assumes position is an integer in ListBox

penguinolog pushed a commit to penguinolog/urwid that referenced this issue Feb 21, 2024
* Fix TreeWalker support in render
* Cover issue with test
* Add initial test for:
* * `TreeNode`
* * `ParentNode`
* * `TreeWalker`
* * `TreeListBox`
* Add basic type annotations for tree widgets

Fix: urwid#828
penguinolog pushed a commit to penguinolog/urwid that referenced this issue Feb 21, 2024
* Fix TreeWalker support in render
* Cover issue with test
* Add initial test for:
* * `TreeNode`
* * `ParentNode`
* * `TreeWalker`
* * `TreeListBox`
* Add basic type annotations for tree widgets

Fix: urwid#828
penguinolog pushed a commit to penguinolog/urwid that referenced this issue Feb 21, 2024
* Fix TreeWalker support in render
* Cover issue with test
* Add initial test for:
* * `TreeNode`
* * `ParentNode`
* * `TreeWalker`
* * `TreeListBox`
* Add basic type annotations for tree widgets

Fix: urwid#828
@penguinolog penguinolog self-assigned this Feb 21, 2024
penguinolog added a commit that referenced this issue Feb 21, 2024
* Fix TreeWalker support in render
* Cover issue with test
* Add initial test for:
* * `TreeNode`
* * `ParentNode`
* * `TreeWalker`
* * `TreeListBox`
* Add basic type annotations for tree widgets

Fix: #828

Co-authored-by: Aleksei Stepanov <alekseis@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants