caesar0301 / treelib

An efficient implementation of tree data structure in python 2/3.
http://treelib.readthedocs.io/en/latest/
Other
801 stars 185 forks source link

migration guide for deprecated bpointer and fpointer #158

Open abulka opened 4 years ago

abulka commented 4 years ago

The latest release 1.6.1 has cause my logs to overflow with deprecation warnings, since the use of node.bpointer and node.fpointer now emits a deprecation warning e.g. DeprecationWarning: Call to deprecated function "fpointer"; use "node.successors" instead. etc.

It took me quite a while to figure how to fix my code to suppress these warnings. Here's how I did it:

node.bpointer  # old
node.predecessor(tree.identifier)  # new

node.fpointer  # old
node.successors(tree.identifier)  # new

At the moment the official treelib documentation still refers to the deprecated properties, with no mention of their replacement methods. Thus anyone who refers to the official documentation to build their code will immediately hit these warnings.

It would be helpful to have a migration guide in the readme or in the doco. And of course the official documentation needs to be updated.

I wonder if it is possible to adjust the deprecation warning so that it is clear that this is not just a mere 'renaming' - it is a change from accessing a property to calling a method - and the method call must have a tree identifier passed into it. That's what took me the time to discover.

vscode debugger triggers deprecation warnings

It seems that visual studio code triggers deprecation warnings - even after I converted my code to the new style. Not sure what is causing this, but I thought I'd report it:

/Users/andy/.vscode/extensions/ms-python.python-2020.3.71659/pythonFiles/lib/python/debugpy/wheels/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_resolver.py:215: DeprecationWarning: Call to deprecated function "fpointer"; use "node.successors" instead.
  attr = getattr(var, name)
/Users/andy/.vscode/extensions/ms-python.python-2020.3.71659/pythonFiles/lib/python/debugpy/wheels/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_resolver.py:215: DeprecationWarning: Call to deprecated function "bpointer"; use "node.predecessor" instead.
  attr = getattr(var, name)

Proposal for enhancement to parameter type

P.S. Having the successors and predecessor be able to take either a tree object or a tree identifier would be helpful IMO since my code has become arguably long winded with this new change e.g. the old style was:

if node.bpointer:
    sibling_nodes = [tree[nid] for nid in tree[node.bpointer].fpointer]

to the new style

if node.predecessor(tree.identifier):
    sibling_nodes = [tree[nid] for nid in tree[node.predecessor(tree.identifier)].successors(tree.identifier)]

The ability to pass in the tree object as a parameter would simplify user's code a little - whilst not being as pithy as the old style, it would be an improvement. Not sure if there are serious performance implications for this change, presumably adding a type check in Node.successors() et. al. would take more time, and possibly slow operations on large trees.

giamas commented 3 years ago

thanks for the information. I spent several our to find a solution and i was lucky to find this posts Unfrotunately documentation for this library it's VERY POOR!

tomol012 commented 2 years ago

If you moved your code to the new style, VSCode will keep throwing deprecation warnings only if in the Run and Debug window, you expand the tree, then nodes, then certain node. This is because for backwards compatibility, the current version of treelib has kept bpointer and fpointer. As a result, when you expand these variables into view, VSCode has to access these properties hence throwing the warning. If you just hide them, VSCode should stop throwing the warning. If it doesn't, chances are you still use it somewhere.

True about the documentation, it's not comprehensive enough - doesn't even touch on several functions of the lib. I use this as a cheat sheet - https://pythonmana.com/2021/12/202112240113053952.html

Could someone explain why the new parameters require tree.identifier to be passed? They are called on a specified node which belongs to the tree you then pass anyway. Can a node belong to several trees?

In general, treelib is awesome, I wish it was kept more active and maintained (especially for pr #180 to be merged)