Some modernization and tidying up #38
Conversation
…reader into use-propub-submodule
also blacken and isort
|
Whoops, super delayed response here, but this is great @fgregg! I've been keeping our branch updated as needed for my purposes but have utterly failed to do much in the way of testing and/or PR any changes back into this repo, so I appreciate this! One general thought I have here is that it's fantastic that the GivingTuesday bucket exists, but since it's not an IRS resource, I wonder if it makes sense to make irsx more data-source-agnostic, with an easy option to toggle the GT bucket as the default data source. This is probably a longer-term conversation (I don't want to hold up updates that get irsx working right out of the box), but something I've been thinking about. |
| # This is the URL to amazon's bucket, could use another synced to it | ||
| IRS_XML_HTTP_BASE = "https://s3.amazonaws.com/irs-form-990" | ||
| # This is the URL to Giving Tuesday's bucket, could use another synced to it | ||
| IRS_XML_HTTP_BASE = env( |
There was a problem hiding this comment.
@asuozzo, to your point about making the XML paths configurable, i made the path to the xml files settable environmental variable.
There was a problem hiding this comment.
yeah, I think that's a great start! My main thought is on long-term reliability of the default setting because we've been burned before, i.e. if the GT bucket gets removed or moves somewhere else, the out-of-the-box setting breaks immediately and requires an override - if the library forces some sort of initial config, then it's less reliant on a presumed default. Hopefully this line of thinking is all theoretical at this point, though!
|
@asuozzo so should your fork be the one to make against? |
Hi Jacob,
this PR
@asuozzo, lmk if your repo is actually the one I should be making a PR against.