Skip to content

Comments

fix device model order#268

Open
antoinefalisse wants to merge 1 commit intodevfrom
device-order
Open

fix device model order#268
antoinefalisse wants to merge 1 commit intodevfrom
device-order

Conversation

@antoinefalisse
Copy link
Collaborator

The iPhoneModel order should take the camera mapping into account since video order might change across trials. Luckily this should not affect processing.

Copy link
Contributor

@carmichaelong carmichaelong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @antoinefalisse, this looks like it should fix the issue that you mentioned.

One change that could be helpful is to make a small helper function for grabbing the clean device id, and then use it in the 3 places in this function (L233, L240, L259). I think the helper function would look something like this (haven't actually tested this):

def getCleanDeviceID(video):
  return video["device_id"].replace('-', '').upper()

Also if you had a quick test that could check this that would be great, but given that this is a small patch to a complicated function, I'd be OK with skipping a test for now for this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants