search

openpreserve / jpylyzer

JP2 (JPEG 2000 Part 1) validator and properties extractor. Jpylyzer was specifically created to check that a JP2 file really conforms to the format's specifications. Additionally jpylyzer is able to extract technical characteristics.
http://jpylyzer.openpreservation.org/
Other
69 stars 28 forks source link

REFACT - String constants #140

Closed carlwilson closed 4 years ago

carlwilson commented 4 years ago
carlwilson commented 4 years ago

@bitsgalore this isn't to be merged yet as it will require some automated testing to ensure that nothing has changed / broken....

bitsgalore commented 4 years ago

I'm not overly in favour of these changes, mainly because I think they will make the maintenance of the MIX module unnecessarily complicated. E.g. suppose a user reports a problem with the reported value of element mix:colorSpace in a jpylyzer output file. Currently it is easy to find the corresponding code with a simple text search on mix:colorSpace. If the proposed changes were applied, we would 1. need to open mix_constants.py, 2. search for mix:colorSpace there, 3. make a note of the corresponding constant MIX.COLOR_SPACE, and then 4. look that up in mix.py. It would also make the MIX code inconsistent with boxvalidator.py, which also uses string constants that are identical to the reported output elements in the XML. So I would suggest to stick with the old string constants, even if they're not completely in line with PEP and/or PyLint.

carlwilson commented 4 years ago

Not a problem at all @bitsgalore that's why I kept them seperate. The one definite improvement is that it means that string constants aren't repeated but I agree with much of your argument. I'll close this for now.