Implements tomlkit in place of toml dependency

[LexiconCode]

Description

Implements tomlkit place of uiri/toml dependency.

A lot of credit goes to @comodoro for testing and breathing some life into the old issue.

Contains temporary a patch in place of dictation-toolbox/dragonfly#125.

Related Issue

Original Pull Request #372

Fixes non-ASCII characters in clipboard
#357

Implements sorting keys
#323

Motivation and Context

uiri/toml is not being maintained enough for the purposes of our project.
String dump with correct escaping #244 pull request by comodoro has been open for 111 days without comment from the author. Thus the motivation to move to an alternative toml parsing library. Along with the related issues mentioned above.

How Has This Been Tested

• Deleting and regenerating all toml files
• Pasted built-in unit tests
• creating/using aliases
• launching bring me commands

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue or bug)
• New feature (non-breaking change which adds functionality) (sorting keys)

Checklist

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• I have checked that my code does not duplicate functionality elsewhere in Caster.
• My code implements all the features I wish to merge in this pull request.
• All new and existing tests pass.

Maintainer/Reviewer Checklist

• Basic functionality has been tested and works as claimed.
• New documentation is clear and complete.
• Code is clear and readable.

[LexiconCode]

One would expect dep_missing() To inform the user of tomlkit missing dependency with instructions when users upgrade to the latest master branch.

However since dependencies.py utilizes castervoice.lib import settings tomlkit tries to load before dep_missing()can execute. Disappointing that this isn't covered but tomlkit is the only third-party module to my knowledge that loads before dep_missing() runs.

[LexiconCode]

@comodoro one thought would be to add some unit tests to writing/reading to toml files.

[comodoro]

Well, you can maybe read requirements (isn't setup.py better as more lenient?) without settings, using __file__, but it would surely need reorganization of dependencies.py. Speaking of which, I can see some problems in the file, like line 21 (is always true), line 46 (sets default timeout, not just for one socket), line 104 (hardcoded dependency, different from setup.py). And it wants to update despite me having latest master. Updates to some 0.6.13 - when current master has version.version 0.6.11. Is there some other branch on pypi?

[LexiconCode]

Thanks for the feedback!

Well, you can maybe read requirements (isn't setup.py better as more lenient?) without settings, using __file__

That's a thought and should be possible.

Speaking of which, I can see some problems in the file, like line 21 (is always true)

if sys.platform == "linux" or "linux2": line 21 is always true?

line 46 (sets default timeout, not just for one socket

good catch socket.setdefaulttimeout(timeout)

line 104 (hardcoded dependency, different from setup.py)

This is intentional ["dragonfly2", ">=", "0.13.0", None], is the bare minimum to run Caster. Using anything lower will crash Caster on startup. This is used not only as an example but for those who choose not to utilize setup.py

[LexiconCode]

Updates to some 0.6.13 - when current master has version.version 0.6.11. Is there some other branch on pypi?

Yes the versions are out of sync. You have to bump the package version each to update pypi. There were few gotchas running the PIP install next to the Classic install. Imports will always come from the PIP install even when you're running the classic install. Except for testing purposes I recommend uninstalling PIP if you have the classic install.

The versions will be put back in sync once we have a method for the automating the changelog.

[comodoro]

It seems that x == 'a' or 'b' is True if the condition in True else 'b'. I thought in the latter case it would be bool('b'), i.e. True. Anyway, you want either sys.platform.startwith("linux"), or sys.platform == "linux" or sys.platform == "linux2"

I thought there was some Context thing later, was it 0.14.1? Anyway, maintaining yet another dependency setting makes things harder.

As for the pip package, I suppose you can just increase the version in the file for each release, or I suppose that's the way they usually do it. Otherwise it seemed to work, are there more problems?

Edit: Accidentally edited when I meant to quote a reply. :/

[LexiconCode]

@comodoro

It seems that x == 'a' or 'b' is True if the condition in True else 'b'. I thought in the latter case it would be bool('b'), i.e. True. Anyway, you want either sys.platform.startwith("linux"), or sys.platform == "linux" or sys.platform == "linux2"

Will do

I thought there was some Context thing later, was it 0.14.1? Anyway, maintaining yet another dependency setting makes things harder.

True enough. However how should we handle the case would've had dependency is changed to something version specific between releases?

The crux of the issue is for the classic install setup.py isn't used to manage dependencies beyond the initial install. The alternative is to simply state that there's been a version dependency change within chat however that's cumbersome and gets buried quickly. This not only handles notifying users by giving them instructions automatically. Although dragonfly minimum version is a rare case and should of been updated to 0.14.1. I might just comment out that line for an example.

If you have other thoughts on how to handle this without the function altogether by all means let me know.

As for the pip package, I suppose you can just increase the version in the file for each release, or I suppose that's the way they usually do it. Otherwise it seemed to work, are there more problems?

That's the plan I just want to be able to show the changes between each release. For now I could do a version bump without explanation and a change log. Otherwise everything else seems to check out.

[LexiconCode]

@comodoro what you think?

def internet_check(host="1.1.1.1", port=53, timeout=3):
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    try:
        s.settimeout(timeout)
        s.connect((host, port))
        return True
    except Exception:
        return False

[comodoro]

I would consider it cleaner with a more specific exception, not sure which, but it is probably fine.

No idea about the dependency issue, what I would probably do is limit dependency checking in non-pip install to what can be reasonably simply coded, in this context the line does not look so bad, else OTOH another file with this kind of more relaxed dependencies could be parsed by both setup.py and dependency checking

[LexiconCode]

I would consider it cleaner with a more specific exception, not sure which, but it is probably fine.

Done f3781bb

...limit dependency checking in non-pip install to what can be reasonably simply coded...

That's already implemented in

Caster/castervoice/lib/ctrl/dependencies.py

Line 140 in f3781bb

if install is "classic":

[LexiconCode]

@comodoro
Reimplemented dep_missing without caster settings dependency

The last tricky bit. Thoughts on how to handle this?

Caster/castervoice/lib/ctrl/dependencies.py

Line 143 in f3781bb

if settings.SETTINGS["miscellaneous"]["online_mode"]:

I suppose I could try except importing settings. On Import error set to true?

[comodoro]

That's tricky. If you want requirements checking at the very beginning and use this setting for update checking, I suppose you have to separate the two and use the requirements checking before loading settings. Sorry, I did not understand the last line.

[LexiconCode]

That's tricky. If you want requirements checking at the very beginning and use this setting for update checking, I suppose you have to separate the two and use the requirements checking before loading settings.

All right have get something worked out for that.

Sorry, I did not understand the last line.
No problem, Your feedback has been very helpful!

Disappointing that this isn't covered but tomlkit is the only third-party module to my knowledge that loads before dep_missing() runs.

Unfortunately this assumption is wrong. 'settings', utilities and control.nexus load before dep.initialize(). hmm

[LexiconCode]

@comodoro
Let me know what you think of #681 Dependency Man loads before first and has been removed from the Nexus.