DM-35278: Increase version of ptcDataset and make it backwards compatible #217
Conversation
| @@ -440,6 +440,9 @@ def fromTable(cls, tableList): | |||
| inDict['badAmps'] = [] | |||
| inDict['photoCharge'] = dict() | |||
|
|
|||
| calibVersion = metadata['PTC_VERSION'] | |||
There was a problem hiding this comment.
Do you need a check on calibVersion for this warning message? It seems like that message should only trigger if calibVersion == 1.0.
There was a problem hiding this comment.
Yes, good catch, thanks. I had the message inside the loop, with the check, but when I decided to just have one message outside the loop, I did not put a check.
| @@ -471,6 +473,11 @@ def fromTable(cls, tableList): | |||
| inDict['finalMeans'][ampName] = record['FINAL_MEANS'] | |||
| inDict['badAmps'] = record['BAD_AMPS'] | |||
| inDict['photoCharge'][ampName] = record['PHOTO_CHARGE'] | |||
| if calibVersion == 1.0: | |||
There was a problem hiding this comment.
Can you add a docstring to the class that mentions that v1.1 adds the ptcTurnoff attribute? I think it'll be easier to understand changes if it doesn't require looking for code tests on calibVersion.
There was a problem hiding this comment.
Agree. On a more general note, are the docstrings the only places where version changes are tracked? I just want to make sure that we don't have to update another document (or perhaps this is the time to decide if we want o create + maintain such an external document).
There was a problem hiding this comment.
I don't know of a better place, so I think adding the changes to the docstring is the minimum we should be doing. I don't think we'll have so many of these that we'll need to have major release notes.
| @@ -471,6 +473,11 @@ def fromTable(cls, tableList): | |||
| inDict['finalMeans'][ampName] = record['FINAL_MEANS'] | |||
| inDict['badAmps'] = record['BAD_AMPS'] | |||
| inDict['photoCharge'][ampName] = record['PHOTO_CHARGE'] | |||
| if calibVersion == 1.0: | |||
There was a problem hiding this comment.
I don't know of a better place, so I think adding the changes to the docstring is the minimum we should be doing. I don't think we'll have so many of these that we'll need to have major release notes.
No description provided.