Fixed indexed-repeat argument path evaluation in integer column type

[gushil]

Closes #484 Closes #507

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

Previous solution (PR #485) with added evaluation for integer type column.

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.

No.

Before submitting this PR, please make sure you have:

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

[codecov-io]

Codecov Report

Merging #499 (211d575) into master (94dc5a2) will decrease coverage by 0.00%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #499      +/-   ##
==========================================
- Coverage   83.86%   83.85%   -0.01%     
==========================================
  Files          25       25              
  Lines        3693     3691       -2     
  Branches      860      859       -1     
==========================================
- Hits         3097     3095       -2     
  Misses        452      452              
  Partials      144      144              

Impacted Files	Coverage Δ
pyxform/survey.py	92.87% <100.00%> (-0.03%)	:arrow_down:

---

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 94dc5a2...211d575. Read the comment docs.

[lognaturel]

This doesn't look right. It will keep failing for every type other than text or integer which is not what we want. I think the difference in behavior is between calculates and non-calculates so perhaps that condition should be if context["type"] != "calculate". But I think there's also something wrong with the else branch because it doesn't account for typed calculates. Should it maybe just be return not("indexed-repeat" in context["bind"]["calculate"])?

[gushil]

@lognaturel I've made the changes you suggested and all the tests are passed.

Thanks!

[gushil]

@lognaturel

Paul told me a couple of indexed-repeat expressions that could cause an issue, like indexed-repeat inside an if-else, or indexed-repeat as an expression, or indexed-repeat as an argument in other function like concat(), substr().

I think those cases are already tested, right?

In the previous commit I've already removed the branching statement, please review it.

Thanks!

[gushil]

@lognaturel

Still waiting for your review/resolve.

Thanks!

[lognaturel]

Happy New Year and I'll try to take a look by the end of the week. Please see #507 for a new test to add.

[MartijnR]

Thanks @lognaturel and @gushil!