Move plotter configuration to configuration file

[theomega]

Removes the possibility to provide the configuration strings via the command line and moves the config over to the vpype configuration.

This is still WIP, the README would need to be updated if we decide to merge this.

Closes #2

[abey79]

I like how most of the code disappears 👍🏻

If the intention is to ship the config file with the plugin (so it is both directly usable and serving as an example), I'd choose a different name (like vpype_gcode.toml or something). You will also need to add it to setup.py or MANIFEST.in (or whatever is the right thing with setuptools) so that it's included in the wheels and carried over by pip install.

[tatarize]

While this is likely where things are going, I do sort of like the whole use a ready-made version and then overwrite a couple little minor aspects within that version, so even without a toml file everything could be configured rather tediously.

It might be reasonable to be able to write a toml entry. I checked the vpype code and it's not currently not API allowed but if you defined the entry well you could use vpype with another command to maybe define such a thing. So people could with a vpype command either update their writer by changing the footer or start a new one. Though that would be a little bit of a perversion of the methods here. Since I'd be taking the command and using that command to hack the toml file stored within vpype. Rather than any of the actual approved pipeline functionalities.

[theomega]

So, I tried to add the bundled_configs.toml, but I'm not sure if it will be packaged correctly like that. I sadly have little experience with setup tools. I tried to follow this guide here: https://stackoverflow.com/questions/11848030/how-include-static-files-to-setuptools-python-package

Do you know @tatarize or @abey79 if this is the correct way? How can I test this?

[abey79]

@theomega it looks ok to me but I've been using poetry for this sort of stuff so I'm not sure. One way to be sure is to just create a new virtual environment somewhere (my /tmp/ tends to have quite a few of those), activate it, and pip install /path/to/vpype-gcode. You can then check if the toml is indeed installed and found by the plug-in by just trying a command.

[abey79]

@tatarize totally feasible and totally naughty :) Not convinced that the use case is there though. Most people will want to put the effort to get a setup running and will then stick to it. Editing ~/.vpype.toml with a template/example taken from the readme (which should have one -- easier to access than the actual bundled .toml file) should be the recommended way to go.

[theomega]

@abey79 tried this and it doesn't work, the file is missing in the install. I need to do some digging, except if you see something obvious...

$ cd /tmp
$ python3 -m venv install_test
$ cd install_test
$ source bin/activate
$ pip install ~/Private/penplot/vpype-gcode/
Processing /home/dominik/Private/penplot/vpype-gcode
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting click
  Using cached click-7.1.2-py2.py3-none-any.whl (82 kB)
Collecting vpype
  Downloading vpype-1.3.0-py3-none-any.whl (91 kB)
     |████████████████████████████████| 91 kB 2.0 MB/s 
Collecting numpy
  Using cached numpy-1.19.5-cp38-cp38-manylinux2010_x86_64.whl (14.9 MB)
Collecting attrs<20.4.0,>=20.3.0
  Using cached attrs-20.3.0-py2.py3-none-any.whl (49 kB)
Collecting Shapely[vectorized]<1.8.0,>=1.7.1
  Using cached Shapely-1.7.1-cp38-cp38-manylinux1_x86_64.whl (1.0 MB)
Collecting moderngl<6.0.0,>=5.6.2
  Using cached moderngl-5.6.2-cp38-cp38-manylinux1_x86_64.whl (675 kB)
Collecting multiprocess<0.71.0,>=0.70.11
  Using cached multiprocess-0.70.11.1-py38-none-any.whl (126 kB)
Collecting svgwrite<1.5,>=1.4
  Downloading svgwrite-1.4.1-py3-none-any.whl (66 kB)
     |████████████████████████████████| 66 kB 4.6 MB/s 
Collecting matplotlib<3.4.0,>=3.3.2
  Using cached matplotlib-3.3.3-cp38-cp38-manylinux1_x86_64.whl (11.6 MB)
Collecting PySide2<6.0.0,>=5.15.2
  Using cached PySide2-5.15.2-5.15.2-cp35.cp36.cp37.cp38.cp39-abi3-manylinux1_x86_64.whl (164.3 MB)
Collecting svgelements==1.3.5
  Using cached svgelements-1.3.5-py2.py3-none-any.whl (64 kB)
Collecting setuptools<52.0.0,>=51.0.0
  Downloading setuptools-51.3.3-py3-none-any.whl (786 kB)
     |████████████████████████████████| 786 kB 4.4 MB/s 
Collecting scipy<1.6.0,>=1.5.4
  Using cached scipy-1.5.4-cp38-cp38-manylinux1_x86_64.whl (25.8 MB)
Collecting toml<0.11.0,>=0.10.2
  Using cached toml-0.10.2-py2.py3-none-any.whl (16 kB)
Collecting click-plugins<1.2.0,>=1.1.1
  Using cached click_plugins-1.1.1-py2.py3-none-any.whl (7.5 kB)
Collecting Pillow<9.0.0,>=8.1.0
  Using cached Pillow-8.1.0-cp38-cp38-manylinux1_x86_64.whl (2.2 MB)
Collecting cachetools<5.0.0,>=4.2.0
  Using cached cachetools-4.2.1-py3-none-any.whl (12 kB)
Collecting glcontext<3,>=2
  Using cached glcontext-2.3-cp38-cp38-manylinux1_x86_64.whl (40 kB)
Collecting dill>=0.3.3
  Using cached dill-0.3.3-py2.py3-none-any.whl (81 kB)
Collecting python-dateutil>=2.1
  Using cached python_dateutil-2.8.1-py2.py3-none-any.whl (227 kB)
Collecting pyparsing!=2.0.4,!=2.1.2,!=2.1.6,>=2.0.3
  Downloading pyparsing-2.4.7-py2.py3-none-any.whl (67 kB)
     |████████████████████████████████| 67 kB 8.0 MB/s 
Collecting kiwisolver>=1.0.1
  Downloading kiwisolver-1.3.1-cp38-cp38-manylinux1_x86_64.whl (1.2 MB)
     |████████████████████████████████| 1.2 MB 9.3 MB/s 
Collecting cycler>=0.10
  Downloading cycler-0.10.0-py2.py3-none-any.whl (6.5 kB)
Collecting shiboken2==5.15.2
  Using cached shiboken2-5.15.2-5.15.2-cp35.cp36.cp37.cp38.cp39-abi3-manylinux1_x86_64.whl (956 kB)
Collecting six>=1.5
  Using cached six-1.15.0-py2.py3-none-any.whl (10 kB)
Building wheels for collected packages: vpype-gcode
  Building wheel for vpype-gcode (PEP 517) ... done
  Created wheel for vpype-gcode: filename=vpype_gcode-0.2.0-py3-none-any.whl size=5552 sha256=4d8555ddcb2c5c6f26c8bc8a62e0a395d179a27c65017cbb8edb23194cff2b32
  Stored in directory: /home/dominik/.cache/pip/wheels/17/4e/a0/f1ecce86d00c7d375d382d746faf7cbaa1c1c7d655d6910878
Successfully built vpype-gcode
Installing collected packages: click, attrs, numpy, Shapely, glcontext, moderngl, dill, multiprocess, svgwrite, Pillow, six, python-dateutil, pyparsing, kiwisolver, cycler, matplotlib, shiboken2, PySide2, svgelements, setuptools, scipy, toml, click-plugins, cachetools, vpype, vpype-gcode
  Attempting uninstall: setuptools
    Found existing installation: setuptools 44.0.0
    Uninstalling setuptools-44.0.0:
      Successfully uninstalled setuptools-44.0.0
Successfully installed Pillow-8.1.0 PySide2-5.15.2 Shapely-1.7.1 attrs-20.3.0 cachetools-4.2.1 click-7.1.2 click-plugins-1.1.1 cycler-0.10.0 dill-0.3.3 glcontext-2.3 kiwisolver-1.3.1 matplotlib-3.3.3 moderngl-5.6.2 multiprocess-0.70.11.1 numpy-1.19.5 pyparsing-2.4.7 python-dateutil-2.8.1 scipy-1.5.4 setuptools-51.3.3 shiboken2-5.15.2 six-1.15.0 svgelements-1.3.5 svgwrite-1.4.1 toml-0.10.2 vpype-1.3.0 vpype-gcode-0.2.0
$ find | grep bundled_configs
$

[abey79]

@theomega I think you need to add include_package_data=True in setup.py.

Also, I would move the .toml inside vpype_gcode/. That's where it'll be found according to:

vp.CONFIG_MANAGER.load_config_file(str(Path(__file__).parent / "bundled_configs.toml"))

Finally, for good measure, I'd add an empty __init__.py to make vpype_gcode a proper python package.

Edit: oh you have include_package_data set already actually. Maybe the other fixes will help? Currently bundled_configs is not "package data" strictly speaking because vpype_gcode is not a package and the file is not in there anyways.

[theomega]

Ok, now it works, thanks for the hints @abey79

[abey79]

Some more suggestions:

1) I'd use -c, --config (shorter and I'm always expecting an int after -n) 2) I'd make it optional. 3) I'd add support for a default config, like there is a default device for HPGL.

The user would do:

[gwrite]
default_config = "my_gcode_config"

[gwrite.my_gcode_config]
# ...

So then it's a simple: vpype read input.svg [...] gwrite output.gcode. Omitting -c without defining default_config would lead to an error.

[theomega]

Are you sure with the -c ? The vpype master command also supports a -c and there it means a configuration FILE and not an option. I would rather avoid the -c. Maybe -n is not ideal though, I'm open for alternatives.

Would you define a default config in the bundled config? This way there should always be a default. What I'm struggling with is, for a gcode writer, what should be the default config. We should come up with a very simple default which shows how this is working. The current gcode config is not ideal for this as it has some inch vs. mm quirks if I understand it correctly.

[abey79]

Are you sure with the -c ? The vpype master command also supports a -c and there it means a configuration FILE and not an option. I would rather avoid the -c.

Yeah I realised that. I agree it's not super optimal but there are only 26 letter in the alphabet :) I already have nasty short commands collisions (like -l meaning layers at some places and landscape at some others). I've grow to accept that short version should make sense locally but not necessarily globally. I should I used --configfile for vpype.

We could use tat's original terminology of "profile"? -p, --profile. Nasty clash with write's --page-size but why not. Alternatively: -t, --template.

I think my vote goes for -p, --profile but really I'd only care about consistency (use the same terminology as in the readme) and, for a single-option command, try to use the actual initial as short cut.

Would you define a default config in the bundled config?

No, I wouldn't. I would explain this in the readme. If the user likes one of the bundled config, they can add it as default in their ~/.vpype.toml.

[theomega]

I switched the commandline to -p/--profile and also added the option to have a default.

What do you guys think? Is this something we could merge here from a code and functionality perspective? I'm happy to update the README then...

[abey79]

Looks good to me appart for the minor comments above. This is a very nice improvement IMO, thanks for that 👍🏻

[tatarize]

Looks fine. I wrote this mostly to get abey to integrate this sort of functionality into vpype since there were plenty of people in the discord with gcode needs that weren't being met. Figured out the really nice ways to do that with some template dictionaries. So much of my goal here is to get abey to take the code. If it goes in that direction, and makes his eventual integration easier I'm quite happy to integrate breaking changes etc. And pushed the breaking change version number up like twice, for your other additions.

He's a toml junkie far more than pre-set hard coded settings. If it passes a little testing and the code has gone in the direction he likes I'll be accepting the PR and uploading it to pip.

---

The readme shouldn't be a sticking point. This was like 2-3 hours of code with a pretty specific goal. If the changes are breaking it gets a different increment in the version. But, I have no qualms letting people using <0.1.0 if they used my the original codebase or 0.3.0 after your other PRs. If you want to radically change terms that's also fine. I'll increment the versions when older code might break from the newer iterations.

The only sort of strong functional shifts that are needed are the join elements between parts in the different levels. I was kinda annoyed my json template couldn't pass a json validator. But, that'd be better addressed in a different PR.

[abey79]

I'll definitely integrate it eventually. I still have to decide what I do with with write, eg. if I add gcode behind it or let it live as a standalone command. Or maybe I deprecate write entirely (and let it there not to break people scripts) and split everything into svg, hpgl, gcode, etc. commands.

Anyways, I'm quite happy for it being a bit battle tested as a plug-in (thanks @theomega for that!) and I have a few things I want to tackle first.

[tatarize]

You should write a different type of plugin for both the write and the read commands that let you select their functionality based on the type of operation they are. So rather than just vpype.plugin you'd have vpype.write and vpype.read. Then divide up the functionality within that to make specific types of write operations that check each writer if they can handle the given filetype. You could just ask each writer that is registered in order whether they handle a given extension and filetype.

The limiting problem would be the options you use. Since with click you actually predefine them. So naturally a write function would take different options depending on the corresponding file type. Which makes the functionality there a bit weirder and different but obviously entirely possible.

If you say: write --ppi 72 myfile.svg that would make sense as an svg. But, if you were writing a gcode file that ppi doesn't make a lick of sense. And without the options things would be a bit weird. You'd actually need to switch out the command completely based on the context of the argument for the command. Basically to be let multiple people write a write command with the same API you currently use, but check the context of the command by the first argument's extension to begin with.

I could currently write this as part of the console command things in Meerk40t, but only because, in part inspired by vpype, I basically implemented a lot of the aspects of Click and have a lot more hackability. It seems like there's a good solution to it. Maybe express a filename first. so file myfile.svg write --ppi where the file command would process the myfile.svg and use that to figure out which of the vp.writer() is appropriate for the given file. You could also have some other aspects where you could do specify that you want all available file formats put into a zip file. But, you'd get a different bit of data_type if you write a writer. Namely you'd get the open stream and the data. And you'd have some code to check which writer is the correct on to use, and preselecting that you'd get to use the options of that particular write command.

That might drift a bit away from the actual click functionality. I'm pretty sure I could do so with my code. But, it's intended for a bunch of internal console functions, and some of the CLI writer stuff was too dedicated to command line stuff. You'd basically need the ability to switch out the functions based on the previous command.

[theomega]

I changed the README to explain the new configuration files. Check it out here: https://github.com/tatarize/vpype-gcode/blob/c9762b480f048c0f2a44dd5bfef92bca3a8bcce7/README.md

I also squashed all commits into one.

What do you guys think? Mergable? Did you try it out, does it work?

[tatarize]

Well, I'll do a once over to see if it I can get it to work before merging. Though a plugin at the periphery, is a far cry from core commands. Though it will certainly be helpful to people.

[tatarize]

Okay, version 0.3.0 is up on Pip.