-
Notifications
You must be signed in to change notification settings - Fork 1
Added script for merging word info in the table and an automation script #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Added mergewords.py to add words info into R table - Modified generate_R_table.py so that it calls mergewords.py automatically - Added a bash script (analysis_script.sh) which automates some parts of the preprocessing - Updated README
|
This looks nice! I think once you insert some whitespace as outlined here and address the other minor comments, we should be good to go!! |
don't rememebr what the changes were for the first two (I made them a while ago), for the last one, I changed line 90, which determined which fields of DA1 file are used as keys in the word dict. I wonder, though - the old version was working somehow on the previous things, right?..
… was due to me not adding number to the DA1 file name (and the main script relies on that)
one is present, it adds postfix "-old" to the name, so that DA1 files from potentially different analyses don't interfere with the current one
generate_R_table.py
Outdated
| @@ -1,3 +1,4 @@ | |||
| #!/Users/antonmalko/anaconda/bin/ python | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only work on your computer, have you tried using the path /usr/bin/env python? That should work on all Unix systems. In general, keep in mind that these scripts are definitely used by people outside UMD, so we have to make sure we don't hard-code too much of our specific setup :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, I was mostly coding it for myself - the script also assumes a certain folder structure which I don't specify anywhere. But yeah, in terms of Python interpreter I can try to make it more portable.
Didn't realize you would receive notifications about changes, I was just playing with this code to adapt it better to what I need now =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, I was thinking about another script... We can delete that line altogether; I think, I put it there to debug something and then forgot to remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, I'm a controlling bastard, so yes, I've set up notifications ;)
I recently got a bug report from someone in Brazil, so I think we should definitely try to keep very specific configurations to a minimum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, was that bug report about the script not being able to process the DA1 files produced by Adrian Staub's "robodoc"?
Even if no, the ability to recognize EyeDoctor and robodoc files would be a useful thing to add at some point (the DA1 files produced by the latter miss a couple of columns, which messes your script up, obviously). Maybe I'll dig through this in summer
| # THE TWO LINES BELOW ARE COMMENTED BECAUSE THEY DON'T WORK CORRECTLY. | ||
| # THE LINE BELOW THEM IS A QUICK FIX (ALSO SEE LINE 396). | ||
| #answer_key = dict_from_table(read_table(file_names['Question key filename']), | ||
| # paired=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How long do you think it would take to implement a real fix for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, actually. I am afraid of diving into the script's guts - I remember trying it about a year ago, and I couldn't really get through (mostly because I rely on stepping through the code in the debugger to really understand how it works, and you use generators quite a lot, which makes it harder)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which debugger are you using? According to this, PDB should step through generators!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time I was using PDB in command line. As far as I remember, my problem was that when I wanted to look up certain variables' values at runtime, it would just tell me that it's a generator object. If I ever return to that, I might try some IDE like PyCharm and see whether it can show generator's contents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you're in PDB, you can either convert the generator to a list or call next() on it to get just one item.
As for the bug report, it had to do with empty lines in DEL files, I don't think that's related to Staub's stuff. Here's the commit that aims to fix it: 2f77974
| import re | ||
| # import some aux functions | ||
| from util import * | ||
| from util import read_table, write_to_table, ask_user_questions, is_yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.