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

Stop reversing classifications #836

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clinton-encord
Copy link
Contributor

Introduction and Explanation

For some reason, we reverse the order of our classifications in to_encord_dict.

Can't think of any good reason why we do this. It just results in classifications being in the wrong order in the UI.

  • E.g. if I have nested classifications, the child answer is shown first, then the parent answer.

JIRA

Link ticket(s)

Documentation

There should be enough internal documentation for a product owner to write customer-facing documentation or a separate PR linked if writing the customer documentation directly. Link all that are relevant below.

  • Internal: notion link
  • Customer docs PR: link
  • OpenAPI/SDK
    • Generated docs: link to example if possible
    • Command to generate: here

Tests

Make a quick statement and post any relevant links of CI / test results. If the testing infrastructure isn’t yet in-place, note that instead.

  • What are the critical unit tests?
  • Explain the Integration Tests such that it’s clear Correctness is satisfied. Link to test results if possible.

Known issues

If there are any known issues with the solution, make a statement about what they are and why they are Ok to leave unsolved for now. Make tickets for the known issues linked to the original ticket linked above

Copy link

Unit test report ((Pydantic 2.x)

206 tests   206 ✅  6s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit c39a448.

Copy link

Unit test report (Pydantic 1.x)

206 tests   206 ✅  6s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit c39a448.

@@ -1695,7 +1695,7 @@ def _to_classification_answers(self) -> Dict[str, Any]:
all_static_answers = classification.get_all_static_answers()
classifications = [answer.to_encord_dict() for answer in all_static_answers if answer.is_answered()]
ret[classification.classification_hash] = {
"classifications": list(reversed(classifications)),
"classifications": list(classifications),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need list constructor here as well? It's a list already, from what I can see

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