Label Refactor #1712
Label Refactor #1712
Conversation
This reverts commit a1942cf.
Label grouping
| @@ -38,6 +38,8 @@ def build_charges_for_summary_panel(record: Record) -> ChargesForSummaryPanel: | |||
| @staticmethod | |||
| def _primary_sort(charge: Charge, record: Record): | |||
| charge_eligibility = charge.expungement_result.charge_eligibility | |||
|
|
|||
| charge_dict = {"nma":0,"ine":1,"nnn":2, "nni":3, "ncn":4, "nci":5, "fnn":6, "fcn":7, "fni":8, "fci":9 } | |||
There was a problem hiding this comment.
Could we use an str, Enum and spell these out?
There was a problem hiding this comment.
I can spell out the different charges, but could you elaborate a bit on the benefits of enum vs. a regular dict for this? Thanks!
There was a problem hiding this comment.
It isn't that different - just a bit more robust. I also just prefer not to abbreviate. For example, I don't know what fni means.
| @@ -15,10 +15,10 @@ def build_charges_for_summary_panel(record: Record) -> ChargesForSummaryPanel: | |||
| ] | |||
| eligible_charges_by_date: List[Tuple[str, List[Tuple[str, List[Tuple[str, str]]]]]] = [] | |||
| sorted_charges = sorted( | |||
| sorted(visible_charges, key=ChargesSummarizer._secondary_sort, reverse=True), | |||
| visible_charges, | |||
| #sorted(visible_charges, key=ChargesSummarizer._secondary_sort, reverse=True), | |||
There was a problem hiding this comment.
I prefer to delete all comments as the git history keeps them anyways
|
@KentShikama changes made to utilize an str,Enum with written-out classifications instead of a dict. Removed pre-existing comment about order now each classification is written out. Manually cast Enum back into an int on line 83 due to typecasting errors occurring when using IntEnum |
There was a problem hiding this comment.
Ignore what I said earlier. I finally understand what you were trying to do with the +=. Basically a clever way to append on a prefix. I would prefer if we just built up boolean variables and then had a function at the end instead of int(ChargeList[classification]).
def function(foo, bar, baz):
if foo and bar and baz:
return 7
if foo and bar and not baz:
return 8
|
Reverted the sorting logic back to if statements, changed |
| @@ -5,7 +5,7 @@ | |||
| from expungeservice.models.record_summary import ChargesForSummaryPanel | |||
| from expungeservice.util import DateWithFuture as date | |||
| from typing import Dict, List, Tuple | |||
|
|
|||
| from enum import Enum | |||
There was a problem hiding this comment.
Do we still need this import?
There was a problem hiding this comment.
Nope, thanks for the catch! removing now.
|
You know about https://dev.recordsponge.com/ right? |
Refactored label grouping algorithm as per #1711. As a result, one backend test case was changed to list charge grouping as "Eligible NOW If Balance Paid on case with Ineligible charge".
Changed grouping algorithm to sort by label and case number only, removed secondary date sort to ensure that charges with the same case number are properly grouped. Will re-introduce if users find new sorting less useful.