search

hrecht / censusapi

R package to retrieve U.S. Census data and metadata via API
https://www.hrecht.com/censusapi/
169 stars 30 forks source link

Update getcensus_functions.R #73

Closed ottothecow closed 2 years ago

ottothecow commented 3 years ago

Attempt to fix issue #57 by merging rather than binding columns.

This works in my testing, but I don't know how to test it fully (never worked on an R package before and I mostly use the tidyverse so my Base R is rusty).

I think it does make some of the later code that removes duplicate columns superfluous because duplicated columns get used in the merge by default.

This does mean that the order of variables isn't exactly preserved--any duplicates will be moved to the front of the line after the geography variables. If there was an easy way to identify the geography variables in advance, I suppose you could merge based only on those, but any given census variable should be immutable, so including them in the merge shouldn't matter.

hrecht commented 3 years ago

Thanks for looking into this. The build with your code is erroring, not sure if that's visible to anyone but me. I'll have to look into this when the news slows down (I run this in free time, am a journalist by day.)

ottothecow commented 3 years ago

If I get a chance I will try to see if I can come up with a cleaner solution (at the very least, probably better to check for duplicate variables prior to making the API calls, then the merge doesn't end up trying to merge based on unnecessary variables).

But I think the builds are failing for a different reason. Even the old releases without my changes are failing because the "key" argument is missing. I don't know anything about Travis CI, but I assume there's some way to specify the key in a secret manner--but it probably only works for the repo owner.

hrecht commented 2 years ago

Hello - just wanted to thank you for submitting this and let you know that a fix is in the works, in the dev version now - see

82.