scanner-research / esper-tv

Esper instance for TV news analysis
Apache License 2.0
39 stars 10 forks source link

image montage functionality should be pulled out of identity.py #86

Closed kayvonf closed 5 years ago

kayvonf commented 6 years ago

I'm going to raise "#question" issues as I continue to read through the code. Many of these are just documenting minor observations during the read through that are likely already on someone's todo list if there ever was a clean up. They are not necessarily important or require immediate action.

It would be useful to extract data-independent visualization routines into their own file. For example, in https://github.com/scanner-research/esper/blob/master/app/esper/identity.py, the function tile_imgs is just a helper function to make an image montage. I'm sure something like this is used in many situations in Esper. This is likely better off somewhere else for reuse, e.g., we could consider app/esper/image_util.py for symmetry with spark_util.py.

willcrichton commented 5 years ago

This is now in the prelude