Open data fmt

[IainMcl]

Open data dates are rrequired to be in a specific format. For quarters this is e.g. 2019Q3. Or when month numbers have to be . I have updated qtr to allow for the option of "open" for open data formats. I have also added a qtr_start function similar to qtr_end.

I haven't done any documentation previously, I have updated the top where it seemed appropreate but not 100% sure.

[IainMcl]

Just haveing a quick scan through why the test for the last commit failed and I think it is because I added a new function to the package? I may be wrong but if that is it im not to sure on how to fix it.

[jackhannah95]

Hi @IainMcl,

Unfortunately this isn't a contribution we'll be able to accept, for a couple of reasons. I feel quite guilty as you've clearly invested time and effort in this and have written some good code. Trying to avoid this type of scenario - where we have to decline speculative contributions that are good but don't fit with what's there already - is a big part of why we've set up the contribution guidelines in the way that we have. You're not the first person to miss them and I doubt you'll be the last either. Maybe we need to think about how to display them more prominently.

There are a few R packages which handle quarters presently, and we only wrote our functions because the output formats we needed ("January to March 2020" or "Jan-Mar 2020") weren't catered for by any of them. However, I believe the open data format you need is catered for already by the {zoo} package:

format(zoo::as.yearqtr(lubridate::dmy(01092019)), "%YQ%q")
#> [1] "2019Q3"

In general we're not keen to re-implement things already covered by other packages (unless we can think of an obviously better way of doing them) so, in this case, we'd prefer to leave this out and point people towards {zoo} if they require the open data format.

As far as a qtr_start() function goes, we only hadn't written it because we weren't aware of any need for it. In hindsight, now that we've written qtr_end() already, if I needed that information I would probably lazily do:

format(zoo::as.yearmon(phsmethods::qtr_end(lubridate::dmy(01092019))) - 1/6, "%B %Y")
#> [1] "July 2019"

However, if you feel that having such a function would be beneficial then by all means add it in! The code you've written for it already looks like a good start (although we have the qtr() functions in alphabetical order so I would possibly move it to the end of the script).

I think the best thing to do would be for me to close this PR for now. I'll add you to the Health-SocialCare-Scotland GitHub organisation to allow you to make a branch on this repo (if you're not able to already) and, if you wish to do so, you can add qtr_start() in on that, and open another PR when you're ready.

Travis can fail for a whole load of reasons. In this case I suspect it's because you typed out the documentation without building it, so R hasn't translated it into a help file. To build, you just need to run ctrl + shift + b (you might have to do Build -> Configure Build Tools -> Configure -> Tick Build & Reload to prevent the Rtools pop-up from appearing). You can also run devtools::check() after you've built the package to check that everything's working okay. It's a manual way of checking the same things that Travis checks. We'd get you to write some unit tests as well before merging, but that can be dealt with at the end.

I hope this seems okay. Please feel free to contact me, either on here or via email if you have any questions.

Thanks,

Jack

[IainMcl]

Thanks for your message! There are lots of really helpful things in there. I had spent some time trying to work out quarters baced on the current date (I didn't relise there was a lubridate function) and then had it mentioned to me that this library was available and it fixed most of my problems. ~30 line function to 1 line call.

Also thanks for mentioning the zoo package it seems like the type that could be useful. Again thanks for your message it helped me understand a lot, Iain

On Monday, 2 March 2020, 11:45:40 GMT, Jack Hannah <notifications@github.com> wrote:  

Hi @IainMcl,

Unfortunately this isn't a contribution we'll be able to accept, for a couple of reasons. I feel quite guilty as you've clearly invested time and effort in this and have written some good code. Trying to avoid this type of scenario - where we have to decline speculative contributions that are good but don't fit with what's there already - is a big part of why we've set up the contribution guidelines in the way that we have. You're not the first person to miss them and I doubt you'll be the last either. Maybe we need to think about how to display them more prominently.

There are a few R packages which handle quarters presently, and we only wrote our functions because the output formats we needed ("January to March 2020" or "Jan-Mar 2020") weren't catered for by any of them. However, I believe the open data format you need is catered for already by the {zoo} package: format(zoo::as.yearqtr(lubridate::dmy(01092019)), "%YQ%q")

> [1] "2019Q3"

In general we're not keen to re-implement things already covered by other packages (unless we can think of an obviously better way of doing them) so, in this case, we'd prefer to leave this out and point people towards {zoo} if they require the open data format.

As far as a qtr_start() function goes, we only hadn't written it because we weren't aware of any need for it. In hindsight, now that we've written qtr_end() already, if I needed that information I would probably lazily do: format(zoo::as.yearmon(phsmethods::qtr_end(lubridate::dmy(1092019))) - 1/6, "%B %Y")

> [1] "July 2019"

However, if you feel that having such a function would be beneficial then by all means add it in! The code you've written for it already looks like a good start (although we have the qtr() functions in alphabetical order so I would possibly move it to the end of the script).

I think the best thing to do would be for me to close this PR for now. I'll add you to the Health-SocialCare-Scotland GitHub organisation to allow you to make a branch on this repo (if you're not able to already) and, if you wish to do so, you can add qtr_start() in on that, and open another PR when you're ready.

Travis can fail for a whole load of reasons. In this case I suspect it's because you typed out the documentation without building it, so R hasn't translated it into a help file. To build, you just need to run ctrl + shift + b (you might have to do Build -> Configure Build Tools -> Configure -> Tick Build & Reload to prevent the Rtools pop-up from appearing). You can also run devtools::check() after you've built the package to check that everything's working okay. It's a manual way of checking the same things that Travis checks. We'd get you to write some unit tests as well before merging, but that can be dealt with at the end.

I hope this seems okay. Please feel free to contact me, either on here or via email if you have any questions.

Thanks,

Jack

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.