search

argoverse / argoverse-api

Official GitHub repository for Argoverse dataset
https://www.argoverse.org
Other
839 stars 240 forks source link

Remove duplicate sensor. #237

Closed benjaminrwilson closed 3 years ago

benjaminrwilson commented 3 years ago

ring_front_center was duplicated in the structured config. The duplicate has been removed.

benjaminrwilson commented 3 years ago

@johnwlambert

johnwlambert commented 3 years ago

looks good. can we add a simple unit test that would be broken without this change? or instead use keyword args, that would catch the duplicate?

benjaminrwilson commented 3 years ago

looks good. can we add a simple unit test that would be broken without this change? or instead use keyword args, that would catch the duplicate?

I don't think dataclass supports star expressions. It seems like we should design this in a way that ensures no duplicate members, though. A unit test would work in the short term, but it seems like we might should rethink how to handle this.

johnwlambert commented 3 years ago

@benjaminrwilson i think keyword args would also fix this, since a duplicate keyword arg would be disallowed?

    camera_sensors: SensorSuiteConfig = SensorSuiteConfig(
        "ring_front_center"=SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_front_center"),
        "ring_front_left"=  SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_front_left"),
        "ring_front_right"= SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_front_right"),
        "ring_side_left"=   SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_side_left"),
        "ring_side_right"=  SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_side_right"),
        "ring_rear_left"=   SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_rear_left"),
        "ring_rear_right"=  SensorConfig(RING_CAMERA_HEIGHT, RING_CAMERA_WIDTH, "ring_rear_right"),
        "stereo_front_left"=SensorConfig(STEREO_CAMERA_HEIGHT, STEREO_CAMERA_WIDTH, "stereo_front_left"),
        "stereo_front_right"=SensorConfig(STEREO_CAMERA_HEIGHT, STEREO_CAMERA_WIDTH, "stereo_front_right"),
    )
benjaminrwilson commented 3 years ago

Whoops, I misunderstood at first. I pushed your suggested fix. Thanks, @johnwlambert.