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

Delete unused lru.py #273

Merged
merged 1 commit into from
Dec 16, 2017
Merged

Delete unused lru.py #273

merged 1 commit into from
Dec 16, 2017

Conversation

zuphilip
Copy link
Collaborator

@zuphilip zuphilip commented Dec 9, 2017

No description provided.

@kba
Copy link
Collaborator

kba commented Dec 12, 2017

This one we could actually delete, since it's just a copy of the snippet from http://code.activestate.com/recipes/498245-lru-and-lfu-cache-decorators/.

@zuphilip
Copy link
Collaborator Author

First, I wanted to delete completely, but then I saw that there is code in OLD which is relying on this lru.py: https://github.com/tmbdev/ocropy/search?utf8=%E2%9C%93&q=lru&type= and thus I moved it also to OLD. As you see, these are not strong reasons against deleting completely. To be honest, I have never used any file or code from OLD so far...

@amitdo
Copy link
Contributor

amitdo commented Dec 13, 2017

About the OLD dir. Maybe it should be removed.

Git keeps the history, you know...

@amitdo
Copy link
Contributor

amitdo commented Dec 13, 2017

'ocropus-sauvola' might be useful. See #210.

Consider moving it back to the root dir. It could be used as an alternative to 'ocropus-nlbin'.

@kba
Copy link
Collaborator

kba commented Dec 13, 2017

About the OLD dir. Maybe it should be removed.

Git keeps the history, you know...

That's true but while you can find/retrieve old files in Git if you know how and what to look for, discovering useful deleted code is tedious (e.g. when searching in Github or Google). I do agree that we should prune all the really old stuff not usable anymore but some code like the sauvola implementation or the synthetic training code is still useful to have for study.

@amitdo Feel free to open a PR for the ocropus-sauvola so we can test/adapt it to the current state of code base.

@zuphilip I would really delete lru.py.

@amitdo
Copy link
Contributor

amitdo commented Dec 13, 2017

but while you can find/retrieve old files in Git if you know how and what to look for, discovering useful deleted code is tedious (e.g. when searching in Github or Google).

You can solve this issue by creating a new 'ocropy-unused' repo and move most of the stuff in 'OLD' dir to this repo.

@zuphilip zuphilip changed the title Move unused lru.py to OLD Delete unused lru.py Dec 16, 2017
@zuphilip
Copy link
Collaborator Author

Okay, the file is deleted now. @kba ready to merge?

@kba kba merged commit 0f33318 into ocropus-archive:master Dec 16, 2017
@zuphilip zuphilip deleted the move-lru-py branch December 16, 2017 15:38
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