search

gmggroup / omf-python

Python library for working with OMF files
MIT License
79 stars 20 forks source link

Provide upgrade path from OMF v1 to v2 #38

Open fwkoch opened 5 years ago

fwkoch commented 5 years ago

We will continue to include a file reader that loads legacy OMF versions and writes to current version.

dbrookes96 commented 5 years ago

Has there been much thought into how to support this? I saw there were changes recently for #37 merging geometry into elements. This presumably breaks the reading compatibility for v1 files. Will there be a separate converter to chop up a v1 OMF file directly and move it into a v2 OMF file?

fwkoch commented 5 years ago

@dbrookes96 - You are right about no backwards compatibility. I think there are a couple options:

  1. Bundle v1 as a submodule in v2 with some extra translation functions (despite backwards incompatibility, this translation should be very straightforward). Then, the v2 reader could fall back to v1 and translate to v2 if given an old file.
  2. Put together a standalone file translation executable, that reads v1, translates, and writes v2. This is similar to (1) but separates concerns a bit; like, you explicitly need to use the translation tool if you want to support v1.
  3. Not support file upgrade.

Thinking ahead to a reference C++ client library implementation (this is the next step after the Python prototype is complete), going with (1) means we would need to build the already-outdated v1 in C++ just to support translation. Not ideal but maybe ok? With (2) we have a standalone tool that can be reused, potentially alongside non-python code? But then this translation must be called explicitly. Option (2) could also just put the burden on whoever is holding the OMFv1 file to upgrade, not vendors... Option (3) is a bit harsh, but I think we could also argue v1 was a proof-of-concept, and realistically everyone just needs to move to v2.

I guess my opinion is something like (1) for the Python implementation (since it's really easy, and most of the code already exists), but the initial C++ implementation would only support v2. If v1 upgrade becomes a requirement in C++ too, we could add that to the client library later.

It would be great to get other perspectives and opinions on this, though!