Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Feature/fix candidate test syntax #247

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Jun 8, 2015

  • Make tests discoverable
  • Make them pass

@arowla @LindsayYoung: this patch also changes the way we render election years in the candidates results page. Before, we were filtering on candidate cycles but rendering the range of candidate election years, which can be different--especially after the change we made to candidate cycles earlier today. This patch uses cycles for both filtering and rendering, so that we get consistent results and passing tests. But there's still a potential inconsistency lurking here: we're now rendering cycles data on the search page but election years data on the detail page. These values can be different, since cycles is based on dimcandproperties.election_yr, and election years is based on dimcandoffice.cand_election_yr. Do you all think it would be all right to build cycles and election years from the same column, or should they reflect (potentially) different things?

@LindsayYoung
Copy link
Contributor

I think that election years and cycles need to stay separate. Having a list of election years is especially helpful to see if there was a special election. I like that we list that information on the candidate page.

@LindsayYoung
Copy link
Contributor

It seems odd that dimcandproperties.election_yr and dimcandoffice.cand_election_yr don't line up. Any insight on that @dcpcc1967 or @jwchumley

@jmcarp
Copy link
Contributor Author

jmcarp commented Jun 8, 2015

@LindsayYoung: One option would be to preserve separate cycles and election years columns (I definitely agree that we should do this), but calculate both columns from the same source--e.g. dimcandoffice.cand_election_yr.

@jwchumley
Copy link

I'm not seeing it. Can you give examples?

From: Lindsay Young notifications@github.com
To: 18F/openFEC-web-app openFEC-web-app@noreply.github.com
Cc: jwchumley jchumley@fec.gov
Date: 06/08/2015 05:33 PM
Subject: Re: [openFEC-web-app] Feature/fix candidate test syntax
(#247)

It seems odd that dimcandproperties.election_yr and
dimcandoffice.cand_election_yr don't line up. Any insight on that
@dcpcc1967 or @jwchumley
?
Reply to this email directly or view it on GitHub.

@jmcarp
Copy link
Contributor Author

jmcarp commented Jun 9, 2015

Example: Norman H Reece (H4CA07048) has election years {1998} from dimcandproperties.election_yr and {1994, 1996, 1998} from dimcandoffice.cand_election_yr. The FEC site lists a third set of two-years periods: {1996, 1998, 2000}. When we filter candidates by year and display the years a candidate was active, which of these year sets should we be using?

For a list of candidates with different year sets from dimcandproperties.election_yr and dimcandoffice.cand_election_yr, run

select distinct on (cand_sk)
    cand_sk,
    cand_id,
    cand_nm,
    election_year_agg.election_years,
    cand_election_year_agg.cand_election_years
from dimcandproperties
join (
    select
        cand_sk,
        array_agg(distinct(election_yr)) as election_years
    from dimcandproperties
    group by cand_sk
) election_year_agg using (cand_sk)
join (
    select
        cand_sk,
        array_agg(distinct(cand_election_yr)) as cand_election_years
    from dimcandoffice
    group by cand_sk
) cand_election_year_agg using (cand_sk)
where election_years != cand_election_years
;

@PaulClark2
Copy link

Norman H Reece (H4CA07048) seems to be wrong in cfdm.dimcandproperties. I've asked the DW developers to look at this case.

If comparing to the Viewer, you generally will not see candidate activity prior to 1995-1996. 2000, as two-year period, is included because Mr. Reece's PCC was active during 1999-2000 even though he was no longer a candidate.

arowla added a commit that referenced this pull request Jun 9, 2015
@arowla arowla merged commit 0e175c7 into fecgov:feature/fix-candidate-test-syntax Jun 9, 2015
@arowla arowla removed the in progress label Jun 9, 2015
@jmcarp
Copy link
Contributor Author

jmcarp commented Jun 9, 2015

@PaulClark2 thanks for clarifying! A few follow-ups:

  • Norman H Reese aside, there are a few thousand candidates that have non-identical years from dimcandproperties and dimcandoffice (based on the query above). Is that expected? If the columns are supposed to be different, which one should we use for election cycle filtering on candidates?
  • Should OpenFEC consider a candidate to be active during cycles for which he/she doesn't have an entry in cand_election_years, but his/her PCC is active--in other words, is the behavior of the Viewer the behavior we should keep going forward?

@PaulClark2
Copy link

@jmcarp

  1. I think we have this fixed. I had the developers make a change to the grouping in the load script. I've looked at the change on ING_. We have not made this change to the DB, PROC_, that feeds your PostGreSQL DB. We'll need to coordinate this if you want the changes.

  2. Yes, if the candidate's PCC is still active we would want the public to be able to access the information. So, in this case, yes we would want OpenFec to behave like the Viewer.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants