Fixed duplicate label translations for secondary itemsets and use itextID for selects with choices that have media specified

[DavisRayM]

Based on #467
Closes #464, #463

Why is this the best possible solution? Were any other approaches considered?

The simplest approach(Least amount of modification to already existing functionality). Ensures that we only generate translations for choices in filtered selects in a controlled manner and that itext is used for selects with media

Considered utilizing uuids, but decided otherwise since uuid generation is unpredictable; Translation entries are created/linked in multiple areas i.e _setup_media & _setup_translations using uuids may lead to more duplicate entries and I'm not entirely sure of an easy way to implement uuids and correctly link them to the question elements

What are the regression risks?

None

Does this change require updates to documentation? If so, please file an issue here and include the link below.

None

Before submitting this PR, please make sure you have:

• included test cases for core behavior and edge cases in tests_v1
• run nosetests and verified all tests pass
• run black pyxform to format code
• verified that any code or assets from external sources are properly credited in comments

[codecov-commenter]

Codecov Report

Merging #468 into master will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #468      +/-   ##
==========================================
+ Coverage   83.39%   83.55%   +0.16%     
==========================================
  Files          25       25              
  Lines        3601     3612      +11     
  Branches      836      839       +3     
==========================================
+ Hits         3003     3018      +15     
+ Misses        454      452       -2     
+ Partials      144      142       -2     

Impacted Files	Coverage Δ
pyxform/question.py	92.95% <100.00%> (+0.10%)	⬆️
pyxform/survey.py	92.54% <100.00%> (+1.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50921a8...47072dc. Read the comment docs.

[lognaturel]

Really liking those tests, @DavisRayM, thanks for the simplified PR! My review process is to look at the tests first, then go through the code and if I can't explain a section, comment it out and run the test suite to see what it does. I've identified two code sections that I don't understand and can currently be commented out without any tests failing. Please illustrate the cases they cover with tests if they are indeed needed.

I like the removal of the static_instance prefix but it seems possibly risky so I'd like another reviewer's thoughts on it.

One difference between your prior PR and this one is that the other one updates xform2json to read in secondary itemset translations. I generally consider xform2json experimental and unsupported so I don't think it's necessary to update. I just wanted to flag it in case that was something you intended to do.

[DavisRayM]

Really liking those tests, @DavisRayM, thanks for the simplified PR! My review process is to look at the tests first, then go through the code and if I can't explain a section, comment it out and run the test suite to see what it does. I've identified two code sections that I don't understand and can currently be commented out without any tests failing. Please illustrate the cases they cover with tests if they are indeed needed.

I like the removal of the static_instance prefix but it seems possibly risky so I'd like another reviewer's thoughts on it.

One difference between your prior PR and this one is that the other one updates xform2json to read in secondary itemset translations. I generally consider xform2json experimental and unsupported so I don't think it's necessary to update. I just wanted to flag it in case that was something you intended to do.

I'm glad. I've gone through, commented, and made changes. Regarding the xform2json changes, I think I'll need to research a bit more into the functionality. Currently, with these changes, the secondary itemset translations don't seem to be a problem(the JSON generated version of the specify_other.xls fixture seems to be passing the test). I'll look into it though and open an issue if I spot an issue with the secondary itemset translation on that functionality.

[lognaturel]

I now feel I can fully explain this. It makes a lot of sense and seems low-risk. Thanks, @DavisRayM! It's great to see some redundancy go away.

Sorry if you saw a weird comment about a remaining issue come through -- I wasn't on the latest commit.