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

Python algorithms and related VectorFst methods have inconsistent return values (self or None) #279

Open
dhdaines opened this issue Aug 15, 2024 · 2 comments · May be fixed by #280
Open

Comments

@dhdaines
Copy link
Contributor

dhdaines commented Aug 15, 2024

Some of them modify the FST in-place, others don't and return a new FST. This is not at all reflected in their return values - some in-place operations return self while others return None.

If some in-place operations return self, then they should all return self so that they can be chained, particularly since some of them which currently return None are non-destructive. These are the ones that should return something and currently don't:

  • tr_sort (really important, so you can do fst1.tr_sort(False).compose(fst2.tr_sort(True)) for instance)
  • rm_epsilon (not an in-place operation either! the version in algorithms does return a new FST) SEE BELOW
  • tr_unique

Also the in-place nature isn't documented, but that is fixed by #278

@dhdaines
Copy link
Contributor Author

Note that #278 would have to be updated with these changes ... I'll wait for a review, if you approve both of these then I can merge them into a new PR.

@dhdaines
Copy link
Contributor Author

Worse yet rm_epsilon actually is an in-place operation, but the version in rustfst.algorithms.rm_epsilon returns ... something else?

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 a pull request may close this issue.

1 participant