Initial automatic dsdl compilation

[chemicstry]

This implements automatic DSDL compilation and fixes #153.

Opening this as an incomplete draft PR so see if I'm going in a correct direction.

Todo:

• [x] Load default paths from environment variables.
• [ ] Check for changed source files. Can be revisited later.
• [x] Documentation.
• [x] Remove temporary debug scripts and lines.

[pavel-kirienko]

This is nice. As far as I understand, the key missing bit is removing the need to install the import hook manually --- it should be done automatically when PyCyphal is imported. The values for lookup_directories are to be sourced from CYPHALPATH, and output_directory is similarly initialized from PYCYPHALPATH (with a default), right? We may want to initialize allow_unregulated_fixed_port_id similarly from CYPHAL_ALLOW_UNREGULATED_FIXED_PORT_ID but it is far less important.

If I remember our conversation correctly, there will be no default value for CYPHALPATH (i.e., empty), but for PYCYPHALPATH the default would be either ~/.pycyphal (or some equivalent on non-GNU platforms) or $CYPHALPATH[0]/.pycyphal. Which one should we go with?

[chemicstry]

This is nice. As far as I understand, the key missing bit is removing the need to install the import hook manually --- it should be done automatically when PyCyphal is imported. The values for lookup_directories are to be sourced from CYPHALPATH, and output_directory is similarly initialized from PYCYPHALPATH (with a default), right? We may want to initialize allow_unregulated_fixed_port_id similarly from CYPHAL_ALLOW_UNREGULATED_FIXED_PORT_ID but it is far less important.

Yes, although might I suggest to use CYPHAL_PATH and PYCYPHAL_PATH? Seems a bit more standard, at least from my experience. Unless CYPHALPATH is already in use somewhere else.

Regarding automatically installing import hook, I was abit worried that it could cause some weird heisenbugs, because DSDLs don't have any common prefix/namespace to be isolated from other packages. However, since it's the last in the import handler chain, maybe it's not a big deal?

If I remember our conversation correctly, there will be no default value for CYPHALPATH (i.e., empty), but for PYCYPHALPATH the default would be either ~/.pycyphal (or some equivalent on non-GNU platforms) or $CYPHALPATH[0]/.pycyphal. Which one should we go with?

I suggest ~/.pycyphal, because $CYPHALPATH[0]/.pycyphal is not really intuitive without knowing internals and users might get confused if behavior changes on reordering paths.

[pavel-kirienko]

Yes, although might I suggest to use CYPHAL_PATH and PYCYPHAL_PATH?

Uhm it's fine, but I thought that PATH variables usually come without the underscore? Either way is fine.

Regarding automatically installing import hook, I was abit worried that it could cause some weird heisenbugs, because DSDLs don't have any common prefix/namespace to be isolated from other packages. However, since it's the last in the import handler chain, maybe it's not a big deal?

I think it should be installed by default but we may want a function for the removal of the import hook for those special cases where the default behavior is undesirable. I am strongly in favor of having the hook installed by default with an explicit opt-out because most applications will want this default behavior.

I suggest ~/.pycyphal, because $CYPHALPATH[0]/.pycyphal is not really intuitive without knowing internals and users might get confused if behavior changes on reordering paths.

Okay.

---

If you're free at 19:00 our time today and would like to discuss this briefly, you would be welcome to attend our resurrected weekly call. See details here: https://forum.opencyphal.org/t/weekly-dev-call/173

[chemicstry]

I think it should be installed by default but we may want a function for the removal of the import hook for those special cases where the default behavior is undesirable. I am strongly in favor of having the hook installed by default with an explicit opt-out because most applications will want this default behavior.

I will try to implement removal then.

If you're free at 19:00 our time today and would like to discuss this briefly, you would be welcome to attend our resurrected weekly call. See details here: https://forum.opencyphal.org/t/weekly-dev-call/173

Sorry, I won't be able to join.

[chemicstry]

Removal of the import hook is a bit complicated because it is inserted into a global sys.meta_path variable, which can be modified by other packages, so you can't rely on index to remove later. You could iterate it and remove by typename, but I think it is far cleaner to add an environment variable, which disables the insertion of default import hook. Which is what I've done.

[pavel-kirienko]

[chemicstry]

:D I mean if you think it would be better to add a remove function, then I can do that instead. However, I see it way more complicated. For example if you use multiple import hooks with different configurations then you also need to differentiate between them. Maybe generating a unique id, which is returned as a handle by install_import_hook and can be used for removal later.

Just to be clear, the way it works now is installed by default, but can be opted out with env variable.

I'm trying to fix CI now and this should be ready to go if you don't have any other comments?

[chemicstry]

I'm not sure I understand the CI error TypeError: 'type' object is not subscriptable, seems to be complaining about Optional[bool]. I tried running nox --non-interactive --session demo check_style docs locally and it all passes, but not on CI.

[pavel-kirienko]

I mean if you think it would be better to add a remove function, then I can do that instead. However, I see it way more complicated.

It's fine. I mostly care about the default case.

I'm not sure I understand the CI error TypeError: 'type' object is not subscriptable, seems to be complaining about Optional[bool]. I tried running nox --non-interactive --session demo check_style docs locally and it all passes, but not on CI.

Seems like the stack trace is pointing to the wrong line. The error is two lines above where it says this:

lookup_directories: Optional[list[_AnyPath]] = None,

Replace list with Sequence from typing.

It passes on your machine because you are using a more recent version of Python.

[pavel-kirienko]

Also, can you please update this section: https://pycyphal.readthedocs.io/en/stable/pages/architecture.html#dsdl-support

The source file is here: https://github.com/OpenCyphal/pycyphal/blob/master/docs/pages/architecture.rst#dsdl-support

[pavel-kirienko]

The CI says (in a very roundabout way) that the demo app is failing to start. Here's the relevant bit:

[00:22:53] 2022-08-18 22:17:02,576 11753 INFO     pycyphal: Log config from env var; level: 'INFO'
[00:22:53] Traceback (most recent call last):
[00:22:53]   File "/home/appveyor/projects/pycyphal/demo/demo_app.py", line 13, in <module>
[00:22:53]     import sirius_cyber_corp  # This is our vendor-specific root namespace. Custom data types.
[00:22:53] ModuleNotFoundError: No module named 'sirius_cyber_corp'

[chemicstry]

Oops, the commits linked to issue, not intended. I'm still trying to fix the CI, but we can squash the commits afterwards.

[pavel-kirienko]

@chemicstry FYI, your progress can be accelerated significantly if you run Nox locally. There are some instructions available in the contributor's guide. Just running nox -s test is all it takes (IIRC).

[chemicstry]

@chemicstry FYI, your progress can be accelerated significantly if you run Nox locally. There are some instructions available in the contributor's guide. Just running nox -s test is all it takes (IIRC).

I've used other nox commands (check_style, docs), but test doesn't work on Windows, at very least it needs socketcan.

[pavel-kirienko]

Nox test works on Windows in the CI environment, no SocketCAN needed ¯\_(ツ)_/¯

https://github.com/OpenCyphal/pycyphal/blob/65d48545c65c3b79c01cb427dd5e083d51426eb7/.appveyor.yml#L67

Seems like there's one remaining issue with home directory detection and then we should be good to merge this.

[chemicstry]

Nox test works on Windows in the CI environment, no SocketCAN needed ¯_(ツ)_/¯

🤦 that flew over my head. After some hiccups managed to run tests on Windows. However, you should update ncat.exe as Windows 11 thinks it's a virus. The latest release from https://nmap.org/download#windows works fine (although needs extra bundled DLLs), but I felt that you would like to update binary files yourself instead of a random stranger.

[pavel-kirienko]

you should update ncat.exe as Windows 11 thinks it's a virus

The default security policy of Windows 11 where it would randomly delete your files is also somewhat reminiscent of a virus (╯°□°)╯︵ ┻━┻

[chemicstry]

@chemicstry are we good to merge now?

Yes, I think it's ready now, finally...