florianfesti / boxes

Boxes.py - laser cutting boxes and more
GNU General Public License v3.0
957 stars 343 forks source link

Make better use of part names #644

Open chrysn opened 4 months ago

chrysn commented 4 months ago

So far, part names are only used to place red text labels.

This change stores the names in the parts (the constructor argument was previously ignored; an extra argument is added to new_part to account for its typical usage pattern where drawing happens on the last set part, and a new part is created at the end of drawing).

A second commit uses the data that is now stored to make names accessible in browsers' tooltips. Future changes enabled by this would be making parts more easily accessible in DXF as well, eg. to extract them for further processing into 3D models.

florianfesti commented 3 months ago

I very much like what this is aiming at. I am not that much a fan of the implementation. Let's just make sure we pass the label to the first call of move(), too. add_part might need a bit of tweaking to make sure we don't keep the old empty part with the old label. But that should just be a single line.

chrysn commented 3 months ago

Oh I fully agree that the retronaming is not pretty. I've tried to make all changes such that no generators would be changed, given that boxes.py can also be used as a library and I don't know what the API guarantees there are.

If I start an alternative version of this where there is a mandatory new_part called at the start of a part rather than at the end (which also makes later steps easier), is there a test suite somewhere that I can run that will execute all generators at least with default configuration? (That'd be helpful because I could make it scream the right way until every move start matches its move end). Ideally, is there something that I can run that asserts that the box outputs did not change?

chrysn commented 3 months ago

And because I'll have to deal with the temptation: Is there a rationale for why the move blocks are not implemented using context managers? (The refactoring necessary to do the labels in the first part will note quite warrant that kind of change, but the temptation will be there).

florianfesti commented 3 months ago

I think the main reason is that context managers did not exist in Python when the code was written. Also passing width and height is super annoying. This was necessary in the past as Boxes.py used libcairo. As we now use our own backend, we should be able to calculate the space needed by the pieces and place them correctly accordingly.

florianfesti commented 3 months ago

Wait a second. Part now has both .extents and .transform. This might be easier than I thought. Give me a minute...

chrysn commented 3 months ago

Would you care to give me some more pointers as to what you'd like to have simplified? (I can also start refactoring the label assignments based on giving the part name beforehand, but it seems you have more concrete ideas.)