newsdev / elex

A wrapper for the AP v2 Elections API.
Other
170 stars 53 forks source link

Fix Winning # being saved as Leading # in trend reports #309

Closed achavez closed 6 years ago

achavez commented 8 years ago

Here's the current output from elex senate-trends:

[
    {
        "current": "51",
        "holdovers": "30",
        "insufficient_vote": "0",
        "leading": "23",
        "net_leaders": "0",
        "net_winners": "+2",
        "office": "U.S. Senate",
        "party": "Dem",
        "winning_trend": "53",
        "won": "23"
    },
    {
        "current": "47",
        "holdovers": "37",
        "insufficient_vote": "0",
        "leading": "8",
        "net_leaders": "0",
        "net_winners": "-2",
        "office": "U.S. Senate",
        "party": "GOP",
        "winning_trend": "45",
        "won": "8"
    },
    {
        "current": "2",
        "holdovers": "0",
        "insufficient_vote": "0",
        "leading": "2",
        "net_leaders": "0",
        "net_winners": "0",
        "office": "U.S. Senate",
        "party": "Others",
        "winning_trend": "2",
        "won": "2"
    }
]

Note the duplicate values on leading/won. This PR fixes that duplication and parses the AP's Leading field to leading in the elex output.

eads commented 8 years ago

Thanks Andrew. Hopefully can roll a bugfix release with this tonight. Can you add a failing test for this? The (very minimal) trend report tests are here: https://github.com/newsdev/elex/blob/master/tests/test_trend_reports.py

palewire commented 8 years ago

Erg. This was almost certainly my mistake initially. Sorry.

eads commented 8 years ago

@palewire @achavez WHO WILL WRITE ME A FAILING TEST?!?!

clears throat

http://1.media.collegehumor.cvcdn.com/59/14/12ad840f8f96e508a4e2bc5d654a18b5.gif

palewire commented 8 years ago

What kind of test are you looking for? Validating the parsed data object back against the raw stuff?

eads commented 8 years ago

Good question. That would work great -- I was thinking just a failing test for this particular issue though.

palewire commented 8 years ago

The bug is deep down in the internals, so I don't think there's a surface level way to test it other than to compare the finished winning and leading numbers, which could in some cases be identical.

achavez commented 8 years ago

@eads: Something like 46bb61fd75c636d6ca1b1c5bc9355bc5f4de2bf0? That's written against the fixtures in the repo. It may be better to ensure that winning + leading = winning trend?

eads commented 8 years ago

Shit, I got behind on this. Ummm, hope to get to it tomorrow probably not tonight but maybe.

eads commented 8 years ago

My apologies, we didn't get to this before the appointed time.