Refactor operations on the EDSL's AST using the visitor pattern

[danoneata]

This pull request continues the refactoring started in #92: it moves the logic initially defined in the data type (as methods, e.g., to_tikz) to corresponding submodules (e.g., backend/tikz.py) using the visitor pattern (this roughly corresponds to pattern matching and fold of the EDSL's abstract syntax tree using an OOP style). It's not cleanest or easiest-to-read approach, but it does help in terms of modularity. (At some point I want to document a bit the library's internals and the various decisions we have made, since some of them are not exactly obvious.)

@srush a couple of questions:

• What is your opinion on this approach? Do you think the gains in terms of modularity trump the potential obfuscation of the implementation? Should I proceed with extracting the rendering of the Shapes to the corresponding backends, as you've previously hinted?
• I'm having some trouble with a type error; maybe you have a suggestion:

chalk/core.py: note: In member "compose" of class "BaseDiagram":
chalk/core.py:65:40: error: Argument 3 to "Compose" has incompatible type "Diagram"; expected
"BaseDiagram"  [arg-type]
            return Compose(envelope, self, other if other is not None else Empty())

[danoneata]

I think I've fixed the typing error described above. Now I'm running into issues when subclassing the Visitor to instances that require extra arguments; for example:

chalk/backend/tikz.py: note: In member "visit_primitive" of class "ToTikZ":
chalk/backend/tikz.py:27:5: error: Signature of "visit_primitive" incompatible with supertype "DiagramVisitor"
[override]
        def visit_primitive(
        ^
chalk/backend/tikz.py:27:5: note:      Superclass:
chalk/backend/tikz.py:27:5: note:          def visit_primitive(self, diagram: Primitive) -> List[Any]
chalk/backend/tikz.py:27:5: note:      Subclass:
chalk/backend/tikz.py:27:5: note:          def visit_primitive(self, diagram: Primitive, pylatex: Any, other_style: Style) -> List[Any]

A similar issue is discussed in this thread and it appears that a solution would be to have default values for the extra arguments (pylatex, other_style), although this might not always make sense. I'll see if I can find any other alternative.

[srush]

Thanks Dan, I think this looks like a really nice PR. I agree that it adds some complexity, but it will untangle some knots and make the core part of the code much simpler. In particular I didn't like having the rendering dependencies in core of the library.

Is it possible that we can avoid the variadic methods by having a second template argument in addition to A for the argument the functions take? If they take 2 maybe that becomes a tuple? (however, I think we don't need that for most cases)

    def visit_compose(self, diagram: Compose, args: B), -> A:

Also just to note: pylatex: PyLatexElement is actually pylatex: PyLatex and it is the module itself. it no longer needs to be passed as an argument and can just be import pylatex. I did it that way because I wanted it to be a conditional dependency.

I like the idea of writing up some better documentation about the structure of the library once we feel like it is stable. Maybe we could even write a talk/paper and send somewhere like PyData.

[srush]

Oh also, let's not factor out the Shape rendering yet. I have another PR coming that will remove almost all the core shapes and replace with a better Trail / Path object. In theory we should only need to render paths and maybe text separately. However, I am a bit stumped by how different backends render arcs at the moment, so it might take a couple more days.

[danoneata]

Is it possible that we can avoid the variadic methods by having a second template argument in addition to A for the argument the functions take?

Yes, I was also thinking something along those lines, but I was afraid that the code would become too unwiedly, so I took a simpler approach in the end: I have moved the mandatory "global" arguments (e.g., dwg or pylatex) as attributes to the Visitor class and specified default values for the arguments that change during the traversal (e.g., style = Style.empty()).

Also just to note: pylatex: PyLatexElement is actually pylatex: PyLatex and it is the module itself. It no longer needs to be passed as an argument and can just be import pylatex. I did it that way because I wanted it to be a conditional dependency.

I've corrected PyLatexElement to PyLatex, but I kept the module as an instance attribute of the ToTikZ visitor such that we still have a conditional dependency. Hope it is okay!

I like the idea of writing up some better documentation about the structure of the library once we feel like it is stable. Maybe we could even write a talk/paper and send somewhere like PyData.

Sounds great! Looking forward to this!

By the way, I've created an organization page for the project (I am not very comfortable that the project is tied only to my name as you probably have contributed more than I did): https://github.com/chalk-diagrams I guess we can transfer the repository once we are done with this large refactoring.

Oh also, let's not factor out the Shape rendering yet.

Okay! Then I think I've mostly done what with this PR.

However, I am a bit stumped by how different backends render arcs at the moment, so it might take a couple more days.

Yeah, I remember my own struggles with the arc_between function. Let me know if you think I can help in any way!