add support for reading .j2 config files with jinja

aelanman commented 2 years ago

@andrerenard I've updated the requirements and marked this PR ready for review

jrs65 commented 2 years ago

Is it worth trying to centralise the parsing of Jinja files into configs that kotekan will read? It would be nice to avoid the issue of having potentially divergent schemes for doing this parsing.

This would require solving the issue that kotekan simply embeds its processing into kotekan.cpp.

andrerenard commented 2 years ago

Perhaps this is a worth a wider discussion on the call this week, but I'm hesitant move to a model which calls python first to parse things and then calls kotekan. It could be done, but it creates some potential issues around how logging and debuggers/GPU profilers work. In most production situations kotekan runs as a service and it really has no need for python, and there is no reason to run it through a wrapper. So I could see being in a world where we end up with different files to run in different modes. If we just want to make sure we are parsing the same way (not that we are doing anything fancy yet) we could just call the python/scripts/config_to_json.py script in the kotekan repo which is the source for the kotekan.cpp call.

jrs65 commented 2 years ago

If we just want to make sure we are parsing the same way (not that we are doing anything fancy yet) we could just call the python/scripts/config_to_json.py script in the kotekan repo which is the source for the kotekan.cpp call.

I would suggest doing this:

Create a python module as part of the kotekan python package that has routines for doing all the parsing.
Adding a if __name__ == "__main__" block to it such that it runs directly as a script without needing the package to be installed.
Have coco install the kotekan python package and use the API from the new parsing module.
Have kotekan either run the python script from an installed location in the fs, or have CMake embed the script at build time in a .h file, and have kotekan compile the string version of the script into the binary from that file (but otherwise run as it currently does).

I think that can be made to conveniently work in all envisaged cases from a single implementation.

However, we might want to pause on merging this PR until we've figured out what to do as in the case above the logic ends up exclusively in the kotekan python package.

jrs65 commented 2 years ago

Agree. That we should merge the aioredis branch first. Currently it fails a bunch of the linting and doc building tests though.

andrerenard commented 2 years ago

@jrs65 I really like that proposal, and I'll also vote we implement this as a python package in the kotekan repo. That also makes it easier and more consistent for projects that want to use something other than coco for system control.

aelanman commented 2 years ago

Agree. That we should merge the aioredis branch first. Currently it fails a bunch of the linting and doc building tests though.

@jrs65 It looks like none of these linting errors on that PR are from my changes. Should I still address them all?

aelanman commented 2 years ago

@andrerenard @jrs65 I've adjusted this accordingly with the changes in kotekan. I set the requirements.txt file to install the version of kotekan on the ael/jinja branch so the tests could pass, but I'll need to set that back to develop once that branch is merged. https://github.com/kotekan/kotekan/pull/1030

jrs65 commented 2 years ago

@aelanman if you update this now #242 is merged I can take another look.

aelanman commented 1 year ago

@andrerenard @jrs65 I thought this was merged earlier today, but was mistaken. Is there anything else needed for this to be merged?

chime-experiment / coco

add support for reading .j2 config files with jinja #246