davidskalinder / mpeds-coder

MPEDS Annotation Interface
MIT License
0 stars 0 forks source link

Make `event_creator_single_value` functionality more flexible #131

Closed davidskalinder closed 3 years ago

davidskalinder commented 3 years ago

See https://github.com/davidskalinder/mpeds-coder/issues/119#issuecomment-885299919 for some of the logic of this. The upshot though is that this is a hard-coded global that specifies which annotation variables should only have one value, with the default being multivalue.

There are several problems with this:

Not sure of the optimal solution for this. As noted in https://github.com/davidskalinder/mpeds-coder/issues/119#issuecomment-885299919, I think the list is permissive, so the fastest solution is simply to extend it so it covers both our and @alexhanna's single-valued vars (though this needs testing). That wouldn't solve any of the issues above, but it might be all that's doable by the time I leave the project.

alexhanna commented 3 years ago

Ah yeah, that's... a good idea. If there's a way of doing this in the YAML templates, that'd possibly be better. That way both the variables and the number of values they can take are defined there. WDYT?

davidskalinder commented 3 years ago

Yeah it should definitely be doable in the YAMLs, and that wouldn't be too bad. What would really be nice though is if MAI could figure out on its own how many values a thing has so that the info doesn't have to live in two different places...

davidskalinder commented 3 years ago

46b35f78e972 provides a temporary fix for this by having the list specified in the config file. This should be enough to conform the codebase to upstream without screwing up each project's settings; then @alexhanna of course if you want to pretty up the process in the future by moving it to YAMLs then it shouldn't be too difficult?

alexhanna commented 3 years ago

sgtm

davidskalinder commented 3 years ago

Deployed and used over the weekend with no apparently problems, so I think this is done except for the longer-term project of putting the config in YAMLs. So I guess I'll close this and let @alexhanna open a new one for that if she wants?

alexhanna commented 3 years ago

Yeah that's fine. It should probably be in YAML but this is low priority for me rn.

On Mon, Aug 30, 2021 at 9:51 AM davidskalinder @.***> wrote:

Deployed and used over the weekend with no apparently problems, so I think this is done except for the longer-term project of putting the config in YAMLs. So I guess I'll close this and let @alexhanna https://github.com/alexhanna open a new one for that if she wants?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davidskalinder/mpeds-coder/issues/131#issuecomment-908506778, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGC6DNBSL3JAIY53C7EUADT7OZJBANCNFSM5A27DK3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Alex Hanna, PhD alex-hanna.com Senior Research Scientist | Google Lecturer | UC Berkeley School of Information