Open alchem0x2A opened 4 years ago
Glad you're finding the library useful!
From what I understand, in version 0.0.6 you need to add a node
figurefirst:mplmethods
inside thefigurefirst:axis
node. Is that correct?
Yes, the documentation is incorrect great catch! We should fix this! However I believe the correct hierarchy should have the figurefirst:mplmethods
node under the svg rect node as is generated by the extensions.
Also there're some side thoughts, I propose to add another functionality FigureLayout.set_axis_props
I love this idea, particularly if we can implement it without calling eval
as we do in the apply_mpl_methods
function.
I'm a little on the fence regarding the idea of using the flatter hierarchy in the SVG - In general I think that flat is better than nested, and cognitively I can see how it might make sense to have the properties be attributes of the figurefirst:axis
node.
That said, the pattern we have for most of the other 'additional function tags' is to add them as their own node directly under the svg
node they apply to. I think that might give a little better separation of concerns such that the nesting is like:
SVG geometry/
├──figurefirst:object_type_indicator
├──figurefirst:function1_indicator
│ └──figurefirst:function1_arg
└── figurefirst:function2_indicator
└──figurefirst:function2_arg
Where function1
and function1
map to functions that would be applied to the figurefirst object type. I'm not sure we adopt this all cases, but I think the majority.
In any case, if we add the function, we should probably also create an extension to decorate the SVG since the original motivation for the library was to create a UI for matplotlib layout design. It's been a while but part of the reason for adopting the above strategy might have related to the ease of writing the extensions.
Curious what others think. A PR would be very welcome!
Thanks for the prompt reply!
I believe the correct hierarchy should have the figurefirst:mplmethods node under the svg rect node as is generated by the extensions.
Indeed. I got the correct way now by using the inkscape extension. Interesting the code seems quite flexible so the nested node also works.
In general I think that flat is better than nested, and cognitively I can see how it might make sense to have the properties be attributes of the figurefirst:axis node.
I think your proposal is cool, although I'm not 100% sure how to distinguish the object_type_indicator
and the function2_indicator
during parsing. I'm not familiar with the svg grammar, so wonder if something like semicolon-separated namespace like figurefirst:methods:some_function_handler
will work.
In any case, if we add the function, we should probably also create an extension to decorate the SVG since the original motivation for the library was to create a UI for matplotlib layout design. It's been a while but part of the reason for adopting the above strategy might have related to the ease of writing the extensions.
That's true. If adhering to the API of 0.0.6 the proof-of-concept approach can be to create a node figurefirst:properties
parallel to the figurefirst:axis
which stores the key:value pairs, and also a similar plugin like the mplmethods.
That's true. If adhering to the API of 0.0.6 the proof-of-concept approach can be to create a node
figurefirst:properties
parallel to thefigurefirst:axis
which stores the key:value pairs, and also a similar plugin like the mplmethods.
I think this is the general organization I was trying to suggest - we haven't really specified the figurefirst grammar anywhere in particular -- so the hierarchy I indicated above was mostly an attempt to generalize what I think is the most common pattern. Perhaps just following how the extensions tag things is the best idea.
Hi
Thanks a lot for the wonderful package! I really enjoyed the workflow of
FigureFirst
and is gradually checking the examples.I think the documentation for the axis methods is somehow broken (https://figurefirst.readthedocs.io/en/latest/examples.html#axis-methods), as the code in the example has no effect to draw an
hspan
.From what I understand, in version 0.0.6 you need to add a node
figurefirst:mplmethods
inside thefigurefirst:axis
node. Is that correct?Also there're some side thoughts, I propose to add another functionality
FigureLayout.set_axis_props
which parse the attributes withinfigurefirst:axis
except reserved names likefigurefirst:name
, as a property ofmpl.axes.Axes
, and set the value using the setter.For instance:
In the current version the hierarchy is:
While in the proposed implementation In the current version the hierarchy is:
I may have several reasons for this: 1) The readability is better than the
mplmethods
2) Explicitly writing theset_
prefix can be omitted, as from my personal usage, most likely themplmethod
of an axis instance is to call a setter of theAxes
class, e.g.ax.set_yticks=[]
for a subplot. 3) In this casemplmethod
can be some functions that solely does simple plotting / patching jobs.I appreciate much your feedback / suggestions.Happy to contribute PRs if you find it reasonable.