search

openstates / enhancement-proposals

Open States Enhancement Proposals
1 stars 3 forks source link

handle VoteEvents that affect multiple bills #12

Open jamesturk opened 4 years ago

jamesturk commented 4 years ago

per opencivicdata/pupa#308

jamesturk commented 3 years ago

pulled in openstates/issues#51 which has some good context

showerst commented 3 years ago

What's your current thinking here for addressing this one?

jamesturk commented 3 years ago

I haven't landed on a solution I'm happy with yet. I want to make it seamless, so the existing interface needs to still work since 99% of the time it is fine.

The simplest option feels like it'd be a new method on VoteEvent -- add_additional_bill() that would take either a Bill object or a chamber/id/etc. I think that'd probably work alright and be minimally disruptive.

As for importing those though, I have no idea-- maybe if a VoteEvent has additional_bills set it shouldnt' have the singular bill set too, since there isn't really one bill that is favored over the rest.

Any thoughts on your end? Is this something you all care about being able to represent in scrapes?

On Tue, Sep 15, 2020 at 11:07 AM showerst notifications@github.com wrote:

What's your current thinking here for addressing this one?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openstates/issues/issues/11#issuecomment-692778168, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB6YVAUIARB6IIHXS4S4DSF57KRANCNFSM4ODD4NGA .

showerst commented 3 years ago

Any thoughts on your end? Is this something you all care about being able to represent in scrapes?

I'd like it in the scrapers, as i'm always a fan of making the data better. I think a method the way you laid it out makes sense.

As for importing those though, I have no idea-- maybe if a VoteEvent has additional_bills set it shouldnt' have the singular bill set too, since there isn't really one bill that is favored over the rest.

Yeah that's a tough one -- on the other hand i feel like no matter what we put in the docs, end users are going to build in an assumption that the VoteEvent always has that (single) bill field set, since exceptions are so rare. Plus leaving it empty all of a sudden would

  1. force consumers to update their code, or at least error out on those particular votes
  2. prevent graceful upgrades

If we were starting from scratch i'd say of course we just make the bills a list -- but I feel like at this point that's a big assumption to overturn, so I guess I'd vote we add a new field to the vote model for additional bills, that doesn't duplicate or empty the first bill.

That way existing code keeps working, but it's fairly obvious there's an additional feature to add even if you're looking at normal vote, since there's a new empty field.

I don't feel super strongly about all this, just my 2c.

jamesturk commented 3 years ago

Adding this to scrapes is fairly easy, I have a draft going here: https://github.com/openstates/openstates-core/compare/multi-bill-votes?expand=1

The work to make these import in the proper way is going to be more difficult than I'd thought though. It might need to even wait until some of the bigger changes to the import process happen. The existing import process uses the singular bill id to detect and remove duplicate votes-- a vote with multiple associated bills will present a challenge there, especially since I imagine there are cases where that list of bills would change. I'm going to put this aside for now, maybe we could even introduce it into the scrape code and import code separately so that others can use it in the meantime, but support in the core data model will have to wait until I have a few more cycles to dedicate to this since I don't want to introduce a lot of likely errors with a rushed job.