returnn icon indicating copy to clipboard operation
returnn copied to clipboard

Potentially unused code

Open JackTemaki opened this issue 3 years ago • 4 comments

I used vulture to get a list of potentially unused code. This would be a good starting point to do further cleaning of the code, in addition to the already existing issues, e.g. #1176.

https://gist.github.com/JackTemaki/6b0453059b9ba5a813125d2b5f1ffdf4

JackTemaki avatar Oct 26 '22 10:10 JackTemaki

Almost all of it looks like false positives.

I think also this issue is too generic. If you find some specific unused code, we should make separate issues about it, but otherwise I'm not really sure how this issue here is helpful.

albertz avatar Oct 26 '22 10:10 albertz

The first ones I checked (in main, setup, log etc...) were actually not false positives, or it was at least not even visible to me how and where the code is used after doing some quick search.

As long as I am the only one working on this, then you are right this issue is not really useful. It was more intended for potentially others to have a starting point.

JackTemaki avatar Oct 26 '22 10:10 JackTemaki

extra_train in __main__: This is valid per Python convention to leave this unused, when it is a tuple result. You often also have this when you iterate over dict items. E.g.:

for k, v in d.items():
  # ignore k, use only v ...

Or similarly:

k, v = get_key_value()
# ignore k, use only v ...

This is valid. I think there is some PEP which explicitly allows this. PyCharm also checks this. Only if you have this:

v = get_whatever()
# v unused

Then this is a warning. So, extra_train is a false positive.

parse_pkg_info was used in an earlier version of the __setup__ script and I wanted to keep it because it was a useful util which I might use again after some refactoring in __setup__. There is a comment describing this. You might argue this can maybe be removed for now and then recovered when we need it again.

StreamThreadLocal was used in some user code, which used wrap_log_streams. But its use was rare, and probably it's not really used actively right now.

But as I said, I think each individual item here probably needs really a separate discussion.

albertz avatar Oct 26 '22 11:10 albertz

Many other things there are probably also used by some users, e.g. MultiEndFastBaumWelchOp, SegmentFastBaumWelchOp. So even if the tool is right that there is no use within RETURNN, it does not tell you that no-one is using it.

albertz avatar Oct 26 '22 11:10 albertz