hrbrmstr / newsflash

Tools to Work with the Internet Archive and GDELT Television Explorer in R
90 stars 9 forks source link

changed logical OR to longer form #4

Closed yeedle closed 7 years ago

yeedle commented 7 years ago

Addresses the bug mentioned in: https://github.com/hrbrmstr/newsflash/issues/3

hrbrmstr commented 7 years ago

ty!

hrbrmstr commented 7 years ago

Actually can you add yourself as a ctb to DESCRIPTION? This will be heading to CRAN in April and I def appreciate taking the time to file and issue and do the PR. It's a solid addition to the pkg. I'll add an Authors@R field in a sec

yeedle commented 7 years ago

Wait, I just realized, this fix will cause an issue a little bit later in the code where you check the value of timespan again, without checking for nullity first. Submitting another pr. Sorry about that.

hrbrmstr commented 7 years ago

no worries. not on CRAN yet so it's in "use at own risk" territory ;-)

yeedle commented 7 years ago

Question: As of now, even if user sets start and end time, timespan of "all" is still used, unless explicitly set to NULL or something else. Is that by design?

hrbrmstr commented 7 years ago

Kinda. I haven't quite figured out the best way to deal with the timespan selection. Def open to ideas here as the lack of a good pkg API is the main reason I haven't even pondered a CRAN release yet. I'm not convinced I've got a good API here (just a functional one) and am def open to any suggestions you may have.

It may mean making multiple query functions (which is totally cool) but I'm still not sure how folks want to engage with the pkg (I do cyber for a living and while I teach data science in my spare time at a few colleges here I'm not an investigative data journalist and rly want to ensure this pkg makes it easy for virtually anyone to find what they're looking for as easily as possible).

yeedle commented 7 years ago

Good point. I've been using it here and there and so far it's been useful. Only suggestion I have so far is the one I tweeted at you

would be nice if can specify keywords and context keywords as vector of strings