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

added bills dataservice #8

Merged
merged 5 commits into from
May 7, 2017
Merged

Conversation

davi17g
Copy link
Contributor

@davi17g davi17g commented Mar 31, 2017

No description provided.

@OriHoch
Copy link
Contributor

OriHoch commented Mar 31, 2017

Amazing!

("name", KnessetDataServiceSimpleField('Name', "string", "bill heb name")),
("type_id", KnessetDataServiceSimpleField('SubTypeID', "integer", "type id of the bill")),
("type_description",
KnessetDataServiceSimpleField('SubTypeDesc', "string", "type description of the bill")),
Copy link
Contributor

Choose a reason for hiding this comment

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

the new knesset API have a lot of these fields where you have somethingID and related somethingDesc
worth to create a new KnessetDataServiceField class which will handle it automatically -
so instead of defining those two fields, you would define only 1 field:
("type", KnessetDataServiceIdDescriptionField("SubType", "type of the bill"))
behind the scenes this field can expose a python tuple of (id, description)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be handled in #13

("num", KnessetDataServiceSimpleField('Number', "Integer",)),
("postponent_reason_id", KnessetDataServiceSimpleField('PostponementReasonID',
"Integer",)),
("postponent_reason_desc", KnessetDataServiceSimpleField('PostponementReasonDesc', "string",)),
Copy link
Contributor

Choose a reason for hiding this comment

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

this is another example of the joined Id/desc field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be handled in #13

("type_description",
KnessetDataServiceSimpleField('SubTypeDesc', "string", "type description of the bill")),
("private_num", KnessetDataServiceSimpleField('PrivateNumber', "integer",)),
("committee_id", KnessetDataServiceSimpleField('CommitteeID', "integer",)),
Copy link
Contributor

@OriHoch OriHoch Apr 1, 2017

Choose a reason for hiding this comment

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

this id field is related to Committee object
for this case you can add a helper method on your object which will try to fetch the related committe
something like this:

def get_committee(self):
  return Committee.get(self.committee_id)

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's worth to create a new class for this type of fields
something like:
("committee", KnessetdataServiceRelatedField("CommitteeID", dataservice_class=Committee))
which will handle it automatically
but need to make sure by default it doesn't fetch the related object - only when user requests it
so I'm not sure what's the best way to implement it..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the get_committee function will be done
regarding the field type, will be done in #14

@davi17g
Copy link
Contributor Author

davi17g commented Apr 2, 2017 via email

@davi17g davi17g requested a review from OriHoch May 7, 2017 16:11
@OriHoch OriHoch changed the title [WIP] added bills dataservice - work in progress added bills dataservice - work in progress May 7, 2017
@OriHoch OriHoch changed the title added bills dataservice - work in progress added bills dataservice May 7, 2017
@OriHoch OriHoch merged commit 6c90631 into hasadna:master May 7, 2017
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