Skip to content

add function replace_nodes to Tree #1524

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

zzc0430
Copy link

@zzc0430 zzc0430 commented Apr 7, 2025

This can be used to replace tokens in a Tree that satisfy a specified rule.

This can be used to replace tokens in a `Tree` that satisfy a specified rule.
@erezsh
Copy link
Member

erezsh commented Apr 7, 2025

Add tests.

@zzc0430
Copy link
Author

zzc0430 commented Apr 8, 2025

Add tests.

added 388fd80

@erezsh
Copy link
Member

erezsh commented Apr 8, 2025

Let's call it replace_tokens.

Also in the tests, you are changing tree2 in-place, which could break other tests if run in a different order.

@zzc0430
Copy link
Author

zzc0430 commented Apr 8, 2025

Let's call it replace_tokens.

Also in the tests, you are changing tree2 in-place, which could break other tests if run in a different order.

done 545b808, caea780

@erezsh
Copy link
Member

erezsh commented Apr 8, 2025

*deepcopy

@zzc0430
Copy link
Author

zzc0430 commented Apr 8, 2025

*deepcopy

5ab7313

@erezsh
Copy link
Member

erezsh commented Apr 8, 2025

@MegaIng Do we really need the special behavior for None? The callback can just return the parameter as is, if no changes are necessary. (I don't think it matters in terms of performance)

@MegaIng
Copy link
Member

MegaIng commented Apr 8, 2025

True, probably not needed.

To get the type checking to work:

  • replace Token in the signature with the Leaf generic parameter.
  • instead of isinstance(..., Token) just use an else statement.

This slightly changes the semantics, but I think it's more useful.

@zzc0430
Copy link
Author

zzc0430 commented Apr 9, 2025

True, probably not needed.

To get the type checking to work:

  • replace Token in the signature with the Leaf generic parameter.
  • instead of isinstance(..., Token) just use an else statement.

This slightly changes the semantics, but I think it's more useful.

c21139c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants