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

Feature/Venue: parametrize review stage #2021

Merged
merged 14 commits into from
Mar 1, 2024

Conversation

celestemartinez
Copy link
Member

@celestemartinez celestemartinez commented Feb 15, 2024

This PR adds two params to the Review Stage:

  • source_submissions_query: the query by which to filter papers
  • child_invitations_name: the name of the children invitations, since it can be different from the super invitation name

invitation_edit_process.py filters papers by source_submissions_query, and creates/updates official invitations only for these papers.

Requires https://github.com/openreview/openreview-api/pull/362

process_path = None
process_path = None,
content_query = {},
child_invitations_name = 'Official_Review'
Copy link
Member

Choose a reason for hiding this comment

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

I think name should represent the child invitation name.

Copy link
Member

Choose a reason for hiding this comment

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

we should have a parent_name that is optional and by default it is the name value

@@ -539,7 +539,9 @@ def __init__(self,
remove_fields = [],
rating_field_name = 'rating',
confidence_field_name = 'confidence',
process_path = None
process_path = None,
content_query = {},
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about calling content_query, it doesn't indicate that is the source of submissions that we want to create the review invitations.

what about:

  • selected_submissions
  • selected_submissions_query
  • selected_submissions_content_query
  • forums_query
  • selected_forums
  • replyto_query
    ...

@@ -65,6 +67,10 @@ def get_children_notes():
if source == 'flagged_for_ethics_review':
source_submissions = [s for s in source_submissions if s.content.get('flagged_for_ethics_review', {}).get('value', False)]

if content_query:
for key, value in content_query.items():
source_submissions = [s for s in source_submissions if value in s.content.get(key, {}).get('value', '')]
Copy link
Member

Choose a reason for hiding this comment

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

should we use in or ==?

Copy link
Member

Choose a reason for hiding this comment

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

I think in is better, it will work for strings and arrays.

@melisabok
Copy link
Member

The changes look good, it is very simple. I made some comments about naming conventions.

@celestemartinez
Copy link
Member Author

@melisabok Great! I can make those changes.

I did find one issue with the ethics review stage:

When a paper is flagged, we update the review invitation and the reviews to include ethics reviewers as readers:

paper_invitation_edit = client.post_invitation_edit(

This is updating the paper invitation using the default super review invitation. How can we denote which super invitation should be used here? This also means that when the ethics review stage is run, we need to somehow get both review forms and update them both, but now only the default one gets updated.

@celestemartinez celestemartinez marked this pull request as ready for review March 1, 2024 14:41
@melisabok melisabok merged commit 4481856 into master Mar 1, 2024
1 check passed
@melisabok melisabok deleted the feature/parametrize-review-stage branch March 1, 2024 19:12
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.

None yet

2 participants