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

[feature] reinstated recursive and added namespace to the results root e... #54

Closed LauraDamianTNA closed 9 years ago

LauraDamianTNA commented 10 years ago

The National Archives has a requirements to validate jp2 files from a particular folder.

bitsgalore commented 10 years ago

Hi Laura,

Many thanks for this. Looks mostly fine, but I think it needs a few minor tweaks. In particular:

  1. Namespace declaration is now only added if the wrapper or recurse flags are true. Wouldn't it make more sense to always add it?
  2. I see you changed the if wrap condition to if wrap or recurse. This essentially means that recurse implies wrap. But then this would be inconsistent with the behavior when running jpylyzer on multiple files without the wrap flag (e.g. jpylyzer *.jp2). So I would suggest to change the condition back to if wrap. (Of course you could question whether anyone would want to run jpylyzer on multiple file without the wrap flag as this doesn't produce well-formed XML.)

I also need to have a better look whether the namespace declaration would qualify as an interface change that could break current deployments. If yes I would need to bump the major release version to 2. That would be fine with me, but I'd prefer to try this out on a separate branch first. Also, if it turns out web have to go for a new major version that might be a good opportunity to flatten out some slight output quirks. As a reminder I just created this issue:

https://github.com/openplanets/jpylyzer/issues/55

My proposal: I just created the new branch recnstna. Could you re-do the pull request to that branch? I can then merge it with that branch and then tweak it a bit more. I will then also have a look if I can incorporate the XSD schema that I created some months ago.

Thanks,

Johan

bitsgalore commented 9 years ago

Closing this pull request after merge with more recent request on recnstna branch: https://github.com/openpreserve/jpylyzer/pull/59