Question about COLOR_BREAK and COLOR_CHANGE

[bastanja]

Hello!

I'm tring to add stitch blocks with different thread colors to an EmbPattern and later retrieve them again as color blocks, e.g. like this:

>>> from pyembroidery import EmbPattern
>>> pattern = EmbPattern()
>>> stitches_1 = [[0,1],[2,3]]
>>> stitches_2 = [[4,5],[6,7]]

>>> pattern.add_block(stitches_1, 0xFF0000)
>>> pattern.add_block(stitches_2, 0x0000FF)

Now when I call pattern.get_as_colorblocks(), I would expect to get two separate color blocks. But I get only a single block:

>>> blocks = pattern.get_as_colorblocks()
>>> print(len(list(blocks)))
1

>>> blocks = pattern.get_as_colorblocks()
>>> block = next(blocks)
>>> print(block)
([[0, 1, 0], [2, 3, 0], [0, 0, 226], [4, 5, 0], [6, 7, 0], [0, 0, 226]], <pyembroidery.EmbThread.EmbThread object at 0x10b1a7ed0>)

The function add_block automatically appends the COLOR_BREAK [0, 0, 226] after the stitches of each block. But the function get_as_colorblocks only splits the blocks at COLOR_CHANGE or NEEDLE_SET. Is this the intended behavior?

I also tried adding a COLOR_BREAK manually between the blocks:

>>> pattern.add_block(stitches_1, 0xFF0000)
>>> pattern.color_change()
>>> pattern.add_block(stitches_2, 0x0000FF)

But this has the effect that the color of the second block is skipped and the second block gets a random third color. This is visible in the exported vp3 file and seems logical, because the COLOR_CHANGE is directly after the COLOR_BREAK and both commands go to the next thread color.

Do you have a suggestion how to add stitch blocks and later retrieve them from the EmbPattern?

What is the difference between COLOR_CHANGE and COLOR_BREAK? Could get_as_colorblocks maybe also split at COLOR_BREAK?

I'm using the pyembroidery version 1.4.12

[tatarize]

First off, I'm kinda jazzed that people are using the software as a proper library. And no. That's not the intended behavior. You're using it as it should work.

...

blocks = pattern.get_as_colorblocks() print(len(list(blocks))) 1

The block it gives you what is clearly (0,1,0), (2,3,0) (0, 0, 226), (4,5,0), (6,7,0), (0,0,226). With the 1 thread object.

But, pattern.threadlist gives you two thread items. And these items certainly do not equal.

Clearly that should have given you color blocks. It built the data correctly.

The function add_block automatically appends the COLOR_BREAK [0, 0, 226] after the stitches of each block. But the function get_as_colorblocks only splits the blocks at COLOR_CHANGE or NEEDLE_SET. Is this the intended behavior?

No. That is not intended. It should also break on color_breaks as well.

if command != COLOR_CHANGE and command != NEEDLE_SET:

The line in 219 should likely read:

if data == COLOR_CHANGE or data == COLOR_BREAK or data == NEEDLE_SET

I also tried adding a COLOR_BREAK manually between the blocks:

Forcing a color change in there, is going to catch the color_change as a real block boundary but also add in a color thread for that change to go to, which will be empty and thus give a random result.

Do you have a suggestion how to add stitch blocks and later retrieve them from the EmbPattern?

Yeah, don't use theget_as_colorblocks() code as it stands. All the rest of the code is fine. It needs to also break at COLOR_BREAK too.

Here's some code to get replace all the COLOR_BREAK with COLOR_CHANGE.

        for stitch in pattern.get_match_commands(COLOR_BREAK):
            stitch[2] = COLOR_CHANGE

Or you could just iterate over the pattern yourself:

        for stitch in pattern:
            print(stitch)

Finding any relevant COLOR_BREAK commands.

Also you could get the normalized version. Calling .get_normalized_pattern().

What is the difference between COLOR_CHANGE and COLOR_BREAK? Could get_as_colorblocks maybe also split at COLOR_BREAK?

Yes, I'll fix that right away. This will only fix one of the issues with that code.

The distinction between color_change and color_break is that color_breaks do not have a distinction within the pattern itself. It's just a breakpoint between one color and the next color. Whereas a color_change refers to a specific operation within the files. You could, in theory start the file with a color_change and properly written, the file should stop at the very beginning. Whereas if you added in a colorbreak it would imply that there's a boundary between one color and the next, but there was no previous color so that should just be ignored. Needle_sets likewise denote a specific command specifically one assigning the needle to the machine for sewing. Doing this at the start of a file is pro forma to ensure it starts on the correct needle, but should not take any special operations. It's just making sure the needle is correct and thus does not denote a colorblock.

While adding in the color_break there to that code will more or less correct the issue for you, it's actually a point where the distinction between commands actually does matter.

---

I'll fix this by tonight. The correct action is actually a bit cryptic.. Because there are points of including the commands. For example a needle_set before the block should be included in the block it's setting the needle for. Whereas the color_change should be at the end of the block since it's ending that color and the colorbreaks should be simply omitted since they convey no information other than there should be a break here.

If anything else in the code has a particularly rough edge go ahead and ask. I've improved rather greatly as a python programmer. And while the writers and readers are spot on, the API could be rougher than would be preferred. Any glaring missing function would be nice to know about as well.

[bastanja]

Thanks for the quick response and the detailed explanation! The difference between color_break and color_change is clearer now.

I tried your fix and I get as many color blocks as expected now. But each color block is missing its last stitch:

>>> from pyembroidery import EmbPattern
>>> pattern = EmbPattern()
>>> stitches_1 = [[0,1],[2,3]]
>>> stitches_2 = [[4,5],[6,7]]
>>> pattern.add_block(stitches_1, 0xFF0000)
>>> pattern.add_block(stitches_2, 0x0000FF)
>>> blocks = pattern.get_as_colorblocks()
>>> block = next(blocks)
>>> print(block)
([[0, 1, 0]], EmbThread(thread='#ff0000'))
>>> block = next(blocks)
>>> print(block)
([[4, 5, 0]], EmbThread(thread='#0000ff'))

The stitches with coordinates [2,3] and [6,7] are not included in the blocks.

Changing the code in get_as_colorblocks to yield one more stitch, i.e. removing the -1 in yield self.stitches[colorblock_start:pos-1] seems to solve it:

        if command == COLOR_BREAK:
            if colorblock_start != pos:
                thread = self.get_thread_or_filler(thread_index)
                thread_index += 1
                yield self.stitches[colorblock_start:pos], thread
            colorblock_start = pos + 1
            continue

But to be honest, I don't know if this breaks anything else in combination with COLOR_CHANGE or NEEDLE_SET and if the index calculation also needs to be adapted in the other if-conditions.

Again, thanks a lot for the quick support! It would be great if you could also check why the last stitches are missing.

[tatarize]

Yeah, that's clearly a bug. Lemme take another whack at it.

I just add new test coverage whenever a new problem rears its head. Then if the fix breaks other things I know pretty quickly.

The blocks needed to have the color_breaks peeled off but it did that with the colorblock_start +1, and the [colorblock_start:pos] is exclusive with regard to the pos element. So I didn't need to subtract one. The bigger risk with the other methods to break the blocks is that I made a similar error. I'll just test to make sure all the resulting block sizes are the equal to the expected ones.

[tatarize]

That should do it with #89. Corrected the bugs with COLOR_BREAK and NEEDLE_SET.

---

If you have any issues with the API or things that seem weird or cumbersome I'd be happy to look at them and fix them. It's a great library and a lot of care was taken to make it good for people directly interfacing with the API. If you find any sharp corners or rough edges bring them to my attention, or even things that make sense but initially were confusing and likely need better documentation.

I'm around and more than willing to make it work for whatever is needed. It's mostly a finished product, and does well what its intended to do. But, I don't run into people with your perspective much, working with it in more a direct API fashion. It works a bit commonly just as the input/output backend. With some of the internal bits working like smart-glue to get it to make the files correctly.

[bastanja]

Thanks for taking care about this! I'll give it a try later today or tomorrow. But I already saw your tests and I'm pretty sure this issue is solved now. Also thank you a lot for the encouragement to report rough edges or missing functions in the API. I've been a happy pyembroidery user for almost a year now without any issues. My main usage is just adding a bunch of stitch blocks and then exporting them as vp3 files. It's good to know that this library is well maintained and open for ideas and improvements!

[tatarize]

Yeah, I'm guessing that that and loading up a bunch of embroidery files and reading them into a format the program needs is the other use case. It's stable enough and powerful enough that it doesn't need much work, but I want it to be a really solid option for anybody who needs this kind of thing. Which means weak docs and rough edges are good enough problems to fix. Code wise my last update before this was a single letter change in DST header, in information that nobody would used. Before that it was not loading discernible but worthless data in a corrupted file header.

[tatarize]

It does what it was intended to do rather perfectly. I might go back and fix the repr calls for the classes. So the classes will have better default values. EmbThread before the update didn't give anything helpful about the thread. Now it's a proper list of what matters. Your opening post gave the thread as: <pyembroidery.EmbThread.EmbThread object at 0x10b1a7ed0> Now they say: EmbThread(thread='#ff0000') which was certainly a rough edge smoothed over.

[bastanja]

So I tried it now and it works perfectly. I also like the new way the thread is printed. Thanks again!

[tatarize]

Using my, now-better-at-python understanding I added #90 as an issue. Are any of those wouldbe api improvements wildly helpful looking. I'd assume the repr for EmbPattern would be a solid helpful thing. Rest are pretty easy, but they clearly help define some of the potential rough edges it has.