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

Make date fields be datetime objects #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

esonderegger
Copy link

@esonderegger esonderegger commented May 31, 2018

Fixes #12

This PR makes the date fields of all the classes in this library be datetime objects and, perhaps more importantly, adds a tests.py file that tests all of these fields.

I looked into putting the parsing logic in utils.py as proposed in the issue. However, I ran into the problem that the OpenFEC API returns a number of formats for its various date fields. I considered adding a dependency on dateutil, but wanted to keep the list of dependencies to a minimum.

Also yield cls(**result) gets called in multiple places in utils.py. It seemed like less repetition to handle conversion from string to datetime in the constructor of every class than to iterate over every key of every result in multiple places in utils.py.

Update (6/15/2018) I just updated this PR to use a utility function for setting attributes of an an instance, provided an object of date fields, with the value being a format string that can be passed to datetime.strptime.

I'm definitely open to doing this another way, though. I'd love to hear your thoughts.

@esonderegger esonderegger force-pushed the 12-datetime-for-dates branch from 8b9cc87 to 92c3dc3 Compare June 15, 2018 16:58
@esonderegger esonderegger force-pushed the 12-datetime-for-dates branch from 92c3dc3 to f4d20ed Compare June 15, 2018 19:33
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.

1 participant