Watts-Lab / commonsense-platform

Commonsense platform
https://commonsense.seas.upenn.edu
1 stars 0 forks source link

Remove hard coded variable names from processing code #111

Open markwhiting opened 5 months ago

markwhiting commented 5 months ago

Things like I_agree in https://github.com/Watts-Lab/commonsense-platform/blob/7c10e1d93cedc5bce39543a3a05445d80d268399/client/src/components/MultiStepForm.tsx#L56 seem like they should be drawn from data on what the questions are in the survey instrument instead of being hard coded values.

markwhiting commented 5 months ago

Following lines include other hard coded arguments for specific values, e.g.,

props.steps[currentStepIndex].answers[2].split("-")[1] === "Yes"

where the index, 2, and the split token - and the value Yes are all hard coded and should probably not be.

markwhiting commented 5 months ago

Another place this can apply is in client/src/data/questions.ts, where we currently have id values which are obscurely mapped to the variable names.

markwhiting commented 3 weeks ago

Was this completed? @amirrr

markwhiting commented 3 weeks ago

The main focus here is to find and eliminate instances of hard coded variables from the main pages of the experiment, instead, these should reference an object that represents the survey, such that treatments can target a particular survey, or even generate them on the fly.

This is complete when no instances of variable names or content from the main survey are in code, and the system can target surveys dynamically. i.e., a good test would be to try targeting a few different surveys and making sure that the correct one shows up and that the data we get out is appropriately annotated.

A more generic goal this works toward is making our system absolutely extensible, so any chances to add tests that help us get closer to that goal are encouraged in completing this!

acao22 commented 2 weeks ago

my branch is on #111-removing-hardcoded-variable-names -- so far I've moved hardcoded variable names to a json file under the data folder surveyData.json and imported the file into the tsx file instead. However, I realized that these hardcoded parts props.steps[currentStepIndex].answers[2].split("-")[1] === "Yes" should be dynamically coded, so working on fixing that now

acao22 commented 2 weeks ago

I also realized that there is already a file questions.tswith the questions json object already defined, so I think it''s better to use that -- I'll edit my code and push again

markwhiting commented 2 weeks ago

Great, make a PR when ready!

acao22 commented 2 weeks ago

just created a pull request!

acao22 commented 5 days ago

I've been trying to solve the failing cypress test for a while now but still not working, I think the format I'm passing the data to the backend is wrong -- what is the standard format? Is it[question id]-[choice]? For example 132-I_agree

amirrr commented 5 days ago

I've been trying to solve the failing cypress test for a while now but still not working, I think the format I'm passing the data to the backend is wrong -- what is the standard format? Is it[question id]-[choice]? For example 132-I_agree

Firstly, thank you for looking into this, the issue might be a bit complicated. You have to change the way we keep answers in states and localStorage perhaps. The object you are passing to the backend is not the right format. It should have this format:

type backendAnswer 
    statementId: number
    I_agree: 1 | 0
    I_agree_reason: string
    others_agree:  1 | 0
    others_agree_reason: string
    perceived_commonsense:  1 | 0
    clarity: 'removed',
    origLanguage: string,
    sessionId: string,
}

I think you can save these keys for each one here, give each object a 'key_string' value with one of column identifiers https://github.com/Watts-Lab/commonsense-platform/blob/84df824f44338267d24addb711ac7ee6b4bc1a09/client/src/data/questions.ts#L10-L19

And then keep track of how we trigger an answer change whenever we change an answer, for example we give the initial values here: https://github.com/Watts-Lab/commonsense-platform/blob/03bd7aa9e8e652075b7bbeae39835b1478aaf306/client/src/components/Layout.tsx#L143-L148

Or here when we change an answer: https://github.com/Watts-Lab/commonsense-platform/blob/84df824f44338267d24addb711ac7ee6b4bc1a09/client/src/components/Statement.tsx#L37-L44

Try to give these a look and feel free to ask questions if you run into any trouble.