Accept other classId in Stroke constructor #346
Conversation
|
The ID seems to be the Can you also come up with a test case? |
I suggested removing the check because it seems like this is the only place where we assert for a specific classID in a constructor. I'll try to generate a new file with this same issue, but the one I have right now has customer data and I can't share it |
Still no luck generating a file with that issue despite trying a few different shapes and versions of this affinity software. Such files definitely exists in the wild. Worst case, I can still get away with a monkey patch if you don't want to accept this PR without a demo file (which I totally understand). |
src/psd_tools/api/shape.py
Outdated
| def __init__(self, data): | ||
| self._data = data | ||
| assert self._data.classID == b'strokeStyle' | ||
| assert self._data.classID == b'strokeStyle' or self._data.classID == b'Strk' |
There was a problem hiding this comment.
@Etienne-Gautier Can you do something like the following?
from psd_tools.terminology import Event
if self._data.classID not in (b'strokeStyle', Event.Stroke):
logger.warning("Unknown class ID found: {}".format(self._data.classID))There was a problem hiding this comment.
OK not to have a fixture file for now
There was a problem hiding this comment.
Sure I'll make that change, I've also just managed to get down to a simple version of the file that reproduces the issue (zipped because github doesn't allow PSDs):
user file cycled.psd.zip
|
The CI error is likely due to the outdated cibuildwheel version. |
Context
I have see a user-uploaded file that psd-tools can't read. This file was produced by Affinity Photo 1.10.4 and exported as PSD.
The file was failing in the
Strokeconstructor atassert self._data.classID == b"strokeStyle". Instead theclassIdisStrkPhotoshop doesn't have a problem opening this file, and does so without warning. So the classId check can be either relaxed (accept 2 values) or removed entirely.
Just wondering if it is best to allow 2 values here, or to remove the assert altogether?