Add custom decision options #2115

enrubio · 2024-04-26T20:44:32Z

Fixes Venue: decision stage should let the PCs decide which decision options are representing an acceptance #1664

Allows custom decision options. ~~Tests still need to be added here.~~

Adds accept_decision_options to request form. If accept_decision_options is empty, we check for the 'Accept' substring in decisions to mark accepted papers. If accept_decision_options is populated, we use those values to mark accepted papers.

melisabok · 2024-04-26T20:51:38Z

Thanks for pushing the code, my recommendation is always starting with the test and then the implementation. This is known as TDD (test development driven) and let you build the feature while you are running it.

Start the decision stage for a venue where the decision options don't contain Accept.
Expect accepted papers to be marked as accepted with venueid == venue group.id

From that you start making the changes.

openreview/venue_request/venue_request.py

openreview/venue_request/process/revisionProcess.py

This reverts commit e42e709.

openreview/venue/process/invitation_edit_process.py

celestemartinez · 2024-05-01T15:52:28Z

openreview/tools.py

        if decision:
            venue += ' ' + decision.strip()
+    elif accept_options and decision_option in accept_options:
+        venue += ' ' + decision.strip()


I think we always should replace Accept right? Some of the accept_options might have the word Accept and we should treat those the same way we used to.

It looks like with this approach, we are doing a combination of checking for 'Accept' and using accept_decision_options?
For example, if I have an option Accept (Oral) in the decision_options, but not in accept_decision_options, it is still treated as an accept?
Not sure if this approach is right or wrong, but want to double check this is the desired behavior?

Yup, basically we only check for "Accept" if accept_decision_options is empty. This is to make it backwards compatible and not let API 1 tests fail and have to make lots of changes

That's correct and that is what is implemented now.

if accept_decision_options then check if decision is in option then marked as accept
else if 'Accept' is in decision then marked as accept

We need to reflect that in the documentation. Correct me if I'm wrong.

ok! I thought this should be the behavior, but from what I see here we are always checking if the decision has 'Accept', regardless of if we have accept_decision_options or not.

Just to be clear @melisabok: a decision of Accept (Oral) that was not specified in accept_decision_options should not be treated as an accept, assuming accept_decision_options is non-empty, correct?

yes, accept_decision_options overwrites the "Accept" string rule.

you are right, the tools implementation should check the accept_decision_options first and then the string rule like in other parts of the code. Emilia could you change it?

celestemartinez · 2024-05-01T15:59:26Z

openreview/venue_request/venue_request.py

                'value-regex': '.*',
                'order': 30
            },
-            'make_decisions_public': {'description': 'Should the decisions be made public immediately upon posting? Default is "No, decisions should NOT be revealed publicly when they are posted".',
+            'accept_decision_options': {
+                'description': 'What are the accept decision options? If your accept options do not contain "Accept", please specify them here. Provide comma separated values, e.g. "Accept (Best Paper), Invite to Archive".',


I know there is an example, but the wording of

If your accept options do not contain "Accept", please specify them here.

makes it sound like PCs should only specify those accept options that do not contain the word Accept, and it could be confusing.

Maybe something like:

What are the accept decision options? Please specify all decision options that signify acceptance to the venue. Any decision option not specified here will be treated as a rejection.

celestemartinez · 2024-05-01T16:03:30Z

tests/test_workshop_v2.py

@@ -360,7 +360,8 @@ def test_publication_chair(self, client, openreview_client, helpers):
            content={
                'decision_start_date': start_date.strftime('%Y/%m/%d'),
                'decision_deadline': due_date.strftime('%Y/%m/%d'),
-                'decision_options': 'Accept, Reject',
+                'decision_options': 'Invite to Venue, Reject',


Can you add an option Accept here and as an accept_decision_option? Also, could you add a check for what the venue value should be for every decision option?

Also, it looks like the homepage is not showing the tab 'Invite to Venue' with the accepted papers. I think there is a get_decision_heading_map() function in helpers that you need to edit. Could you also add this to the test? There is an example here:

openreview-py/tests/test_arr_venue_v2.py

Line 1899 in c4892b1

request_page(selenium, 'http://localhost:3030/group?id=aclweb.org/ACL/ARR/2023/August', None, wait_for_element='header')

enrubio · 2024-05-02T13:34:15Z

I'm unable to get the homepage tab tests working, no idea why. They show up in the browser. I've tried both selenium.find_element(By.CLASS_NAME, 'nav-tabs').find_elements(By.TAG_NAME, 'li') and selenium.find_element(By.CLASS_NAME, 'tabs-container') and neither give the correct tabs.

melisabok · 2024-05-02T14:28:35Z

I can take a look, just to confirm is the same test that is failing in circleci? tests.test_workshop_v2.TestWorkshopV2

tests/test_workshop_v2.py

enrubio · 2024-05-02T17:03:02Z

openreview/venue_request/venue_request.py

            'decision_options': {
-                'description': 'What are the decision options (provide comma separated values, e.g. Accept (Best Paper), Accept, Reject)? Leave empty for default options - "Accept (Oral)", "Accept (Poster)", "Reject"',
+                'description': 'List all decision options. By default, decisions containing "Accept" signify acceptance to the venue. Otherwise, please specify the accept options in "Accept Decision Options". All other decisions will be treated as a rejection. Provide comma separated values, e.g. "Accept (Best Paper), Invite to Archive, Reject". Leave empty for default options - "Accept (Oral)", "Accept (Poster)", "Reject"',
                'value-regex': '.*',
                'order': 30
            },


What do you think about changing the description to just:

List all decision options. Provide comma separated values, e.g. "Accept (Best Paper), Invite to Archive, Reject". Leave empty for default options - "Accept (Oral)", "Accept (Poster)", "Reject"

I'm worried it's too confusing and PCs would fill out the fields incorrectly. For example, if "Accept" and "Invite to Archive" are considered accept options, they may just put "Invite to Archive" in accept_decision_options. I think this would cause even "Accept" papers to be rejected since it's not listed as an accept option. We could also add to the accept_decision_options description something like:

Leaving this empty will treat only decisions with "Accept" as accepted.

can we make the field mandatory and have a default value?

We do use ['Accept (Oral)', 'Accept (Poster)', 'Reject'] as the default options in the request form if they leave the field empty. Do you want this value to be added to the request form?

yes, I think it is more clear for the users to know which are the default options how they can edit them.

Yes I think with the simplified description we don't need to make the fields required. I fixed it and added the default value. I think it's much better now.

melisabok · 2024-05-03T14:48:00Z

openreview/tools.py

-        decision = decision_option.replace('Accept', '')
-        decision = re.sub(r'[()\W]+', '', decision)
-        if decision:
+    decision = re.sub(r'[()\W]+', '', decision_option)


I think this can be inside the if, right?

it seems the condition if (not accept_options and 'Accept' in decision_option) or (accept_options and decision_option in accept_options) is used in several places, can we create a tools function that we use from the venue class, process function and this function too?

Yes that would be great, updated to add the tools function

celestemartinez

Looks great!

Add custom decision options

e42e709

melisabok reviewed Apr 26, 2024

View reviewed changes

openreview/venue_request/venue_request.py Show resolved Hide resolved

melisabok reviewed Apr 26, 2024

View reviewed changes

openreview/venue_request/process/revisionProcess.py Outdated Show resolved Hide resolved

enrubio added 2 commits April 29, 2024 16:22

Revert "Add custom decision options"

9ee5747

This reverts commit e42e709.

Add accept_decision_options to request form

3eac8dc

melisabok reviewed Apr 30, 2024

View reviewed changes

openreview/venue/process/invitation_edit_process.py Outdated Show resolved Hide resolved

Fix default value

a470bc8

melisabok marked this pull request as ready for review May 1, 2024 13:37

melisabok requested a review from celestemartinez May 1, 2024 13:38

melisabok approved these changes May 1, 2024

View reviewed changes

celestemartinez reviewed May 1, 2024

View reviewed changes

enrubio added 2 commits May 1, 2024 14:49

Add decision stage preprocess

b95b7b8

Changes from feedback

b6b0516

melisabok reviewed May 2, 2024

View reviewed changes

tests/test_workshop_v2.py Outdated Show resolved Hide resolved

Fix tests

fa2e376

enrubio commented May 2, 2024

View reviewed changes

enrubio added 5 commits May 2, 2024 14:55

Add default value

f668873

Fix description

afb58b9

Merge branch 'master' into fix/custom-decision-options

4b6f5de

Replace Accept for accept options

42880d2

Fix string

ef1b132

melisabok reviewed May 3, 2024

View reviewed changes

Add tools function to check decision acceptance

065ff3e

celestemartinez approved these changes May 6, 2024

View reviewed changes

melisabok merged commit 3e3a54f into master May 6, 2024
1 check passed

melisabok deleted the fix/custom-decision-options branch May 6, 2024 15:29

enrubio mentioned this pull request May 6, 2024

Venue: decision stage should let the PCs decide which decision options are representing an acceptance #1664

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom decision options #2115

Add custom decision options #2115

enrubio commented Apr 26, 2024 •

edited

melisabok commented Apr 26, 2024

celestemartinez May 1, 2024

melisabok May 1, 2024

celestemartinez May 2, 2024

enrubio May 2, 2024

melisabok May 2, 2024

celestemartinez May 2, 2024

melisabok May 2, 2024

celestemartinez May 1, 2024

celestemartinez May 1, 2024

celestemartinez May 1, 2024

enrubio commented May 2, 2024

melisabok commented May 2, 2024

enrubio May 2, 2024

melisabok May 2, 2024

celestemartinez May 2, 2024

melisabok May 2, 2024

enrubio May 2, 2024

melisabok May 3, 2024

melisabok May 3, 2024

enrubio May 3, 2024

celestemartinez left a comment

Add custom decision options #2115

Add custom decision options #2115

Conversation

enrubio commented Apr 26, 2024 • edited

melisabok commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enrubio commented May 2, 2024

melisabok commented May 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

celestemartinez left a comment

Choose a reason for hiding this comment

enrubio commented Apr 26, 2024 •

edited