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

Implement functions for endpoint: Series #21

Closed
10 tasks done
sboysel opened this issue Jul 12, 2018 · 5 comments
Closed
10 tasks done

Implement functions for endpoint: Series #21

sboysel opened this issue Jul 12, 2018 · 5 comments
Labels

Comments

@sboysel
Copy link
Owner

sboysel commented Jul 12, 2018

https://research.stlouisfed.org/docs/api/fred/#Series

The following endpoints need to be implemented properly and consistently in fredr:

The functions are to be implemented with explicit arguments for each parameter in the FRED API (see documentation for each endpoint and the discussion in #20).

@sboysel sboysel assigned sboysel and unassigned sboysel Jul 12, 2018
@sboysel sboysel added the core label Jul 12, 2018
@sboysel sboysel removed their assignment Jul 12, 2018
@sboysel
Copy link
Owner Author

sboysel commented Jul 14, 2018

@DavisVaughan: Just as a heads up, I'm currently working on these functions in a local feature branch.

@DavisVaughan
Copy link
Collaborator

Sounds good. I see for the required Params you changed the default from nothing to null. I see the reason you did that (so you can pass to the validation function) but I'd like to ensure the user knows it's required ahead of time. We could either

A) add "required" by the parameter in the documentation

B) restructure the argument checking to be

if(is.missing(search_id)) stop(required_msg, call.=FALSE)
validate_required_param(search_id)

@sboysel
Copy link
Owner Author

sboysel commented Jul 14, 2018

The function validate_series_id() will throw the error if the user leaves series_id = NULL.

> fredr_series()
 Error: Argument `series_id` must be supplied. 

I don't think there are any endpoints where series_id is an optional parameter.

@DavisVaughan
Copy link
Collaborator

I guess im saying, if i was a user and was trying to use this pkg for the first time, how do I know what params are required in the function call the first time I use it? having all of the optional params as param = NULL and the required params as param is a clear way to do that. Or just add it in the docs as "required param" and leave param = NULL for the required params.

Just trying to be uber user friendly and not have the only way they can figure out what is or is not required be running into an error (even if its a helpful one!)

@sboysel
Copy link
Owner Author

sboysel commented Jul 14, 2018

I see. I'll go with option A in this PR.

@sboysel sboysel closed this as completed Jul 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants