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

Cleanup from excessive learning and DBUS #25

Closed
rinigus opened this issue Mar 12, 2018 · 5 comments · Fixed by #26
Closed

Cleanup from excessive learning and DBUS #25

rinigus opened this issue Mar 12, 2018 · 5 comments · Fixed by #26

Comments

@rinigus
Copy link
Contributor

rinigus commented Mar 12, 2018

This is kind of 2in1 issues: I suggest to clean up code from:

  • learn is called when the word is accepted by the user. As such, it should not be needed since presage is learning on fly. Method learn is designed for the cases where you give external text and don't care about predictions at that time.

  • we have DBUS interaction and clear method linked to it. As discussed in Add configuration page #5 , I suggest to drop it. When cleaning is needed, we can just close maliit and delete all data from databases. Actually, since its SQLite, there is even no need to close maliit - database can be accessed from multiple processes and we can purge n-grams from the config page. However, the later scenario will result in having out-of-date cached counts for the purged database in maliit RAM.

If you agree, I'll submit PR with such cleanup

@rinigus
Copy link
Contributor Author

rinigus commented Mar 12, 2018

Related: #1

@rinigus
Copy link
Contributor Author

rinigus commented Mar 14, 2018

Additional note: I was thinking that we can stash away DBUS code into separate branch. That way, we can always take it back easily, if needed.

@rinigus
Copy link
Contributor Author

rinigus commented Mar 14, 2018

I have started working on this. Will submit it together with "forget a word" PR

@martonmiklos
Copy link
Collaborator

martonmiklos commented Mar 14, 2018

DBUS cleanup -> acked I do not think that it is going to be useful if we are going to have a separate config page. Having it in a branch for posterity makes sense.

Excessive learning -> acked. How should I say first I have tried to mimic the Xt9 without actually thinking/investigating deeper how the presage works to get the stuff rolling and then it simply left in.

Submit together with the forgot word -> sounds good.

@rinigus
Copy link
Contributor Author

rinigus commented Mar 14, 2018

And that was very easy to fix - done already :) . I'll wait till hunspell is merged and then submit forget PR. Otherwise, forget PR will have the same commits as hunspell and its going to get messy.

PS: when submitted / merged, we can mark this issue as closed. Just to let all know that we have issue fix available, just waiting in the pipe

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.

2 participants