scanner-research / esper-tv

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

change immediates to named constants #88

Closed kayvonf closed 6 years ago

kayvonf commented 6 years ago

Another source of potential future bugs is leaving parameters of an experiment in the code as immediates rather than named constants. For example, consider the immediate 50 on line 229 of app/esper/identity.py.

I'd rather see this as: 50 --> IMAGE_SAMPLES_PER_BUCKET

The file identity.py has three instances of the immediate "50" in it. Two correspond to the number of image samples per bucket, and the third is a default parameter to show_gender_examples. An inadvertent search/replace operation might incorrectly change all three.

This is both for readability and safety.

kayvonf commented 6 years ago

Ditto for the "40" in the construction of the array of major canonical shows.

e.g., 40 --> NUM_MAJOR_SHOWS

jhong93 commented 6 years ago

Now fixed in the current code. :)