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

chore: assessment api methods #2028

Closed
wants to merge 82 commits into from

Conversation

muselesscreator
Copy link
Contributor

TL;DR - [ A short summary of what this PR does and why ]

Initial rip of api logic from non-peer assessment mixins.

@muselesscreator muselesscreator requested a review from a team as a code owner August 14, 2023 20:05
@muselesscreator muselesscreator marked this pull request as draft August 14, 2023 20:06
context, error_message = self._parse_example(self.example)
return {
'error_message': error_message,
'essay_context': context
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "essay" the original wording? Because it would avoid it if it is not, these are frequently (but not always) essays and using that language could cause confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was training_essay_context

@muselesscreator muselesscreator force-pushed the bw/assessment_apis branch 2 times, most recently from dd599e9 to 6ca7344 Compare August 14, 2023 21:31
@muselesscreator muselesscreator force-pushed the bw/assessment_apis branch 2 times, most recently from dd38301 to f74ea26 Compare August 18, 2023 14:48
def _self_path_and_context(self, key, step_data, with_sub=False):
return self.SELF_TEMPLATE_PATHS[key], self.self_context(step_data, with_sub)

def self_path_and_context(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

My preference is to change this from:

  • path & context > identify template > proxy to getting context

To:

  • path & context > (get path & get context) as 2 separate functions (as in submission_mixin)

In the first approach, you're effectively hiding the get_context function. In the second, path and context are both top-level functions that get assembled by the path_and_context function.

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