Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Added AnswerKey class to mturk/question.py #2143

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

iliakur
Copy link

@iliakur iliakur commented Mar 4, 2014

Hi,

I was trying to use this library to upload a qualification test to the server and discovered that there was no AnswerKey class created. I did that and have updated the relevant MTurk connection code.

I'm also interested in continuing to work on the mturk functionality of this package.
I've updated the string formatting to use the more modern .format() method.
I also made some stuff a bit more pythonic and explicit.

items.insert(1, SimpleField('DisplayName', self.display_name))
items = ''.join(item.get_as_xml() for item in items)
return self.template % vars()
return self.template.format(items)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, .format(...) was added in Python 2.6 & there's no compatibility in Python 2.5 for it. We try not to break Python 2.5 in Boto.

@toastdriven
Copy link
Contributor

All the calls to .format(...) would need reverting. Additionally, it looks to me like this change would need unit tests added, though I might be wrong on that. As for the actual functionality, @disruptek, would you mind weighing in? Beyond that, looks good.

@disruptek
Copy link
Contributor

Not sure who you're looking for, but pretty sure it's not me. I'm the MWS/FPS guy. ;-)

@iliakur
Copy link
Author

iliakur commented Mar 13, 2014

I've reverted the formatting commands and will have a look at unit tests in the next couple of days.

@toastdriven
Copy link
Contributor

Gah, sorry for the noise @disruptek.

@budlight
Copy link

budlight commented Apr 6, 2014

@copper-head Did you ever get the unit tests for mturk working? It appears to me that someone changed the file to disable executing the local.py and basically broke the test setup in general. If I revert their change they run fine for me. Kind of rediculous to require tests for a test suite that basically can't work as is....

Edit: looking further the test suite doesn't seem to be running in travis either so why would you want to write a test that nobody will ever use and basically isn't even checked anyway.

@iliakur
Copy link
Author

iliakur commented Jun 17, 2014

Hi folks.

My apologies for the delay in response. Things came up at work. I looked through the unit tests and was unable to locate anything that I thought looked related to mturk. There is, however, an "mturk" directory containing a bunch of tests. However, I'm not sure how exactly this addition would be tested (would we have a look at the output html file to make sure it's sane?). This may be because I don't have any experience with unit testing, but what is our end game here?

@iliakur
Copy link
Author

iliakur commented Jun 17, 2014

One other question. Your contributor documentation says boto supports Python 2.6 and up. Moreover, Python 2.5 isn't even in the official documentation any more. Are we sure we want to keep the % formatting syntax instead of format()?

@danielgtaylor
Copy link
Member

We are dropping support for Python 2.5, however I suggest we keep the % syntax for consistency with the rest of the codebase for now.

As for the unit tests, we'd need some way to confirm that this code works as expected. I believe the tests/mturk directory can be modified to include a test with your new class. Unfortunately these tests are not run often, so I'm not sure of the state of them. If we can get a test added and get confirmation that it works against the service then I'll merge it in.

Also, it looks like you added a bunch of .ropeproject files into this pull request by mistake.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants