Trigger column

[gushil]

Closes #438
Closes #70

[codecov-commenter]

Codecov Report

Merging #447 into master will increase coverage by 0.18%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
+ Coverage   82.93%   83.11%   +0.18%     
==========================================
  Files          25       25              
  Lines        3556     3595      +39     
  Branches      821      833      +12     
==========================================
+ Hits         2949     2988      +39     
  Misses        460      460              
  Partials      147      147              

Impacted Files	Coverage Δ
pyxform/builder.py	77.18% <100.00%> (+1.79%)	⬆️
pyxform/question.py	93.17% <100.00%> (+0.57%)	⬆️
pyxform/survey.py	91.51% <100.00%> (+0.11%)	⬆️
pyxform/survey_element.py	90.17% <100.00%> (+0.08%)	⬆️

---

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 245f5cd...0f1b6b9. Read the comment docs.

[lognaturel]

I did another pass through and everything is still looking reasonable to me. @MartijnR could you please look through the new failure case tests added and see if you think of any others? It'd also be great to get your thoughts on coverage for #70 and the inline question.

[lognaturel]

Here's something else I wanted to mention -- there's no support added to xform2json and little thought has been given to the JSON representation. I think in general it's up to folks who rely on those tools to participate in review or submit the functionality after the fact if they need it.

[MartijnR]

Thank you! Will check it out and am aiming for tomorrow!

[MartijnR]

there's no support added to xform2json and little thought has been given to the JSON representation. I think in general it's up to folks who rely on those tools to participate in review or submit the functionality after the fact if they need it

This makes sense to me. Didn't realize JSON wasn't used internally at all.

Thanks @lognaturel and @gushil!

[lognaturel]

JSON wasn't used internally at all

It is as an intermediary step. If someone actually wanted to use the JSON for something directly, they might need to introduce an index from triggers to calculations. We did that after the JSON representation.

[lognaturel]

Phew, sorry about the force pushes! I'm feeling good about this and if @gushil and @MartijnR agree then I think we can merge. 🎉

[MartijnR]

Yes! :party: Thanks!

[gushil]

Yes, of course! Thanks @lognaturel @MartijnR