Skip to content
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

Fix two backwards compatibility bugs #528

Merged
merged 1 commit into from
Oct 1, 2021
Merged

Conversation

jacob-lee
Copy link
Collaborator

  1. The local config file text says that table_name is being deprecated in favor of assignments_table_name and that assignments_table_name is preferred if both defined. However, only table_name was being used. An entry into the backwards_compatibility list in psiturk_config.py was added for this.
  2. In the psiturk_config.py loop through the backwards compatibilities, the section was being hard-coded to Hit Configuration. This prevented fix in (1) from taking effect. It also was preventing the backwards compatibility setting for the logfiles from taking effect.

1. The local config file text says that `table_name` is being deprecated in favor of `assignments_table_name` and that `assignments_table_name` is preferred if both defined. However, only `table_name` was being used. An entry into the backwards_compatibility list in psiturk_config.py was added for this.
2. In the psiturk_config.py loop through the backwards compatibilities, the section was being hard-coded to `Hit Configuration`. This prevented fix in (1) from taking effect. It also was preventing the backwards compatibility setting for the logfiles from taking effect.
@jacob-lee
Copy link
Collaborator Author

Again test_amt.py is failing.

@deargle
Copy link
Collaborator

deargle commented Sep 28, 2021

Hrm, idk about that test offhand, but that's a lot of failing tests. Concerning. I'll try to debug within the next few days.

]
for bc in backwards_compatibilities:
env_key = f'PSITURK_{bc["prefer_this"].upper()}'
if env_key in os.environ:
self.set(bc['in_section'], bc['over_this'], os.environ.get(env_key))
else:
preferred = self.get('HIT Configuration', bc['prefer_this'], fallback=None)
Copy link
Collaborator

Choose a reason for hiding this comment

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

yikes, how was this ever working!

@jacob-lee
Copy link
Collaborator Author

I cloned master, created a new conda environments (3.6 and 3.7) with master as the pip -e installed version of psiturk, and I am getting matching fails for test_amt.py:

FAILED tests/test_amt.py::TestAmtServices::test_wrapper_get_all_hits - AssertionError: assert '3XJOUITW8URHJMX7F00H20LGRIAQTX' in []
FAILED tests/test_amt.py::TestAmtServices::test_wrapper_get_all_hits_all_studies - AssertionError: assert '3BFNCI9LYKQ2ENUY4MLKKW0NSU437W' in []
FAILED tests/test_amt.py::TestAmtServices::test_wrapper_get_active_hits - AssertionError: assert 'ABCDUITW8URHJMX7F00H20LGRIAQTX' in []
FAILED tests/test_amt.py::TestAmtServices::test_wrapper_extend_hit - AttributeError: 'dict' object has no attribute 'options'
FAILED tests/test_amt.py::TestAmtServices::test_wrapper_approve_all_assignments - assert 0 == 6
FAILED tests/test_amt.py::TestAmtServices::test_wrapper_reject_unreject_assignments - assert 0 == 6

Whatever the problem is, it isn't my patch.

@deargle
Copy link
Collaborator

deargle commented Oct 1, 2021

Since the same code that worked a few months ago now fails, I'm guessing some dependency released a new version that is compatible with psiturk's requirements, which is now breaking things. Looking into it.

@deargle
Copy link
Collaborator

deargle commented Oct 1, 2021

Got'em, closeio/ciso8601#108

@deargle
Copy link
Collaborator

deargle commented Oct 1, 2021

Wait what are we doing, we shouldn't be doing any of this today! Hacktoberfest doesn't start until tomorrow!

@deargle deargle mentioned this pull request Oct 1, 2021
@deargle deargle merged commit 11824f5 into NYUCCL:master Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants