dictation-toolbox / natlink

Natlink provides the interface between Dragon and python
https://dictation-toolbox.github.io/natlink/
Other
25 stars 17 forks source link

INI remove hardcoded paths and add tests #121

Closed dougransom closed 2 years ago

dougransom commented 2 years ago

addresses this issue https://github.com/dictation-toolbox/natlink/issues/118

n branch ini_remove_code_paths , natlink init looks like this

`[packages] disabled_packages = enabled_packages = unimacro,vocola2

[directories] vocolagrammarsdirectory = C:\Users\Doug.natlink\VocolaGrammars unimacrogrammarsdirectory = C:\Users\Doug.natlink\UnimacroGrammars

[userenglish-directories] `

dougransom commented 2 years ago

I think my proposed change is a huge win. Now it doesn't matter where a package is installed natlink will find it.

Initial install as admin, then a pip upgrade without admin privileges could put unimacro in a different folder. No problem, no need to rerun a config program.

This change will also get us to a point where it is humanly possible to edit the ini file. A user/developer can just edit natlink.ini to have

[packages] enabled packages=unimacro instead of

[directories] unimacro=C:\Users\Doug\AppData\Roaming\Python\Python39\site-packages\vocola2

especially if they switch to/from admin installs (which is likely). And if we further support other install areas for python packages (i envision likely, like maybe virtualenvs) developers can switch rapidly between those environments and not have to edit their natlink.ini.

The change I propose lets the python environment do the work of finding where a package is, since it knows. The natlinkconfig only knows at the instant it is run, but it can easily change.

quintijn commented 2 years ago

I feel, after some thinking, more sympathy for you idea.

I see two possibilities

  1. the loader and/or the config program has knowledge of directories, that should be loaded, or
  2. with my initial idea, the path to eg. unimacro is like %(sitepackages)/unimacro . The loader/config module should then be able to resolve this place. Or even only unimacro. Other packages might not need a setting in the directories section.

Bot OTOH, the loader could read a packages section, when defined, they are enabled, maybe, like [packages] unimacro = unimacro vocola = vocola2 dragonfly =

(empty, when no directory has to be loaded.)

So first the packages, after that the additional directories, for such a package, defined by the config program.

dougransom commented 2 years ago

Packages other than unimacro and vocola don't seem to need a directory/package specified in natlink.ini — i presume the user-area scripts just import what they need. Unimacro and vocola probably have some startup stuff that must be loaded hence the need to specify a directory where they are located.

Specifying a package is basically specifying a directory to be found along the python path.

Why do you prefer: [packages] unimacro = unimacro vocola = vocola2 dragonfly = over

[packages] enabled packages=unimacro, vocola2?

In practice likely only vocola2 and unimacro will be listed in packages.

It shouldn't be terribly hard to change from the way I did it to the way you propose (one line/package). All the unit tests are built with pytest.

quintijn commented 2 years ago

The main reason for Doug's packages idea is simplification of the natlink.ini file, and unnecessary confusing paths that may change (eg. site-packages/unimacro for another version or in another place), where python can resolve the path.

OTOH for the loader the process should be as simple as possible. Just now, we can introduce 2 sort of environment variables, which are handled by the config/loader modules:

sitepackages and natlink_userdir.

%(sitepackages)/unimacro for example indicates python has to find the current unimacro in all possible site-packages.

%(natlink_userdir)/UnimacroGrammars points to the current directory where the (active) unimacro grammars are found, parallel to natlink.ini itself. (Defined by NATLINK_USERDIR, or ~/.natlink when this environment variable is empty.

Do we think of the natlink.ini file as manually edited, or set by the config program?

Advantage of specifying packages is, that checks on updates can be done (possibly an option to the loader).

Concluding: I am not against the idea of packages, but the main issue seems to be the possible changing paths issue as pointed out above. I think that part is resolved by these ideas...

quintijn commented 2 years ago

Oops, I think the notation is $(sitepackages)... or $sitepackages..., not %(...)...

dougransom commented 2 years ago

As much as possible I would like to reduce reliance on environment variables for config. Just one to point to an alternate natlink.ini should be enough.

any other settings should be in the ini file. I intend to eventually add settings to enable debuggers to connect to natlink python and maybe even natlink C++.

We don't need a sitepackages variable; python already knows a bunch of paths to search and for a users who is also develops there could be several: appdata/... c:/program files.../site-packages, anything else they have set up.

What could be interesting is a pythonpath setting in natlink.ini, where paths listed are prepended to the pythonpath when natlink starts. But if anyone really needed that they can edit an environmental variable. It is an edge case that someone will need this.

It seems we are in agreement to specify a package rather than a folder when it makes sense to. I think though that @quintijn's proposed format like this: [packages] unimacro = unimacro vocola = vocola2 dragonfly =

is better because it is consistent with the directories setting, just relying on python to resolve the full path.

I will make the change and nudge for you to consider the PR then.

dougransom commented 2 years ago

Do we think of the natlink.ini file as manually edited, or set by the config program?

I don't know but I would like it to be easy at least for developers to edit the file to make/test changes. It is becoming a lot easier now the natlink registration is taking care of by the installer and the settings are pretty much in one place.

quintijn commented 2 years ago

I think the config program should do most of the config process, at least for most users. But manual editing should be also as clear as possible!

quintijn commented 2 years ago

So...

The packages entry should be additional the directories section. The directories should be complete, but possibly with the package directory being loaded if that package requires it (unimacro and vocola, for now).

In the startup process the packages entries can be used for checking, and possibly updating, new releases. When an entry only specifies a package name (unimacrodirectory = unimacro), python can resolve the directory. I am working on that in the "ini_remove_code_paths" branch of the NatlinkDoug fork.

Moreover, I need a special for the extra directories like unimacrogrammarsdirectory = natlink_userdir\UnimacroGrammars. As I wrote before. No such clever trick as with a directory in a site-packages directory unfortunately.