cdelker / schemdraw

MIT License
103 stars 20 forks source link

`drawing.container` padx, pady and cornerradius are not functional #32

Open SidaChen1999 opened 5 months ago

SidaChen1999 commented 5 months ago

From the source code, I found that the initializing parameters padx, pady and cornerradius are never used. However, the get_bbox function used the padding parameters from self.params['padx']. I'm not sure where this dictionary comes from but changing the initializer to the following works for me.

def __init__(self,
                   drawing: Union['Drawing', 'Container'],
                   cornerradius: Optional[float] = None,
                   padx: Optional[float] = None,
                   pady: Optional[float] = None):
        super().__init__()
        self.drawing = drawing
        self.elements: list[Element] = []
        self.params['padx'] = padx
        self.params['pady'] = pady
        self.params['cornerradius'] = cornerradius
cdelker commented 5 months ago

How are you creating and using the container? The parameters work as expected when used in this example.

The three parameters are automatically added to Element.params on instantiation so the lines you added are unnecessary, and in general setting params in an init is not advisable because it's a ChainMap and may mess up the prioritization/inheritance of these values. It could be you're doing something unexpected to instantiate the Container which bypasses adding them in.


Update - IF you're not using the container in a context manager like in the example, AND you're passing the parameters as positional, not keyword, then they could be ignored. I suggest a better fix is to force them to be keyword-only parameters, to align with how all the other elements have been set up:

def __init__(self,
             drawing: Union['Drawing', 'Container'],
             *,
             cornerradius: Optional[float] = None,
             padx: Optional[float] = None,
             pady: Optional[float] = None):
SidaChen1999 commented 5 months ago

Hi cdelker,

I used it as following:

with schemdraw.Drawing(unit=1) as d:
    flow.Start().label('Start')
    flow.Arrow().down().length(1.5)
    with d.container(padx=3) as c:
        flow.Box().label('Step 1').drop('E')
        flow.Arrow().right()
        flow.Box().label('Step 2')
        c.color('red')
        c.label('Subprocess', loc='N', halign='center', valign='top')
    flow.Arrow().right()
    flow.Start().label('End').anchor('W')

closed by accident

SidaChen1999 commented 5 months ago

closed by accident

cdelker commented 5 months ago

Looks to be working as expected. What are you getting compared with what you're expecting? Note the container will not change the position of its contents, it just draws a box around them.

image

SidaChen1999 commented 5 months ago

emmm, looks like maybe it's the environment issue image