-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
mctq: An R Package for the Munich ChronoType Questionnaire #434
Comments
Thanks for submitting! @jooolia will be the Handling editor. |
Editor checks:
Editor commentsThanks for the submission. I did some initial automated checks on the spelling, recommended practices, and the code coverage and everything looks shipshape! I will now proceed to looking for reviewers. Thanks! Julia |
@ropensci-review-bot seeking reviewers |
Please add this badge to the README of your package repository: [![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/434_status.svg)](https://github.com/ropensci/software-review/issues/434) Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news |
Hi @danielvartan, |
Hi @danielvartan, I am still actively seeking reviewers. If you have any suggested reviewers I can also take these into account in my search. |
Hi @jooolia, Thank you for all of your time and effort. There are few people I know in the fields of sleep and chronobiology who work with R. The first person that crossed my mind was @leocadio-miguel (Mario André Leocadio Miguel | ORCID: https://orcid.org/0000-0002-7248-3529). Mario and I have worked on some projects together. I don't know if this may be an issue for this kind of peer-review. I will try to think of other possible candidates. |
@ropensci-review-bot add @leocadio-miguel to reviewers |
That can't be done if there is no editor assigned |
@ropensci-review-bot assign @jooolia as editor |
Assigned! @jooolia is now the editor |
@ropensci-review-bot add @leocadio-miguel to reviewers |
That can't be done if there is no editor assigned |
@ropensci-review-bot add @leocadio-miguel to reviewers |
@leocadio-miguel added to the reviewers list. Review due date is 2021-05-26. Thanks @leocadio-miguel for accepting to review! Please refer to our reviewer guide. |
@ropensci-review-bot add @jonkeane to reviewers |
@jonkeane added to the reviewers list. Review due date is 2021-05-27. Thanks @jonkeane for accepting to review! Please refer to our reviewer guide. |
Hi @jonkeane, I'm very impressed with the extent and quality of your review. Thank you very much for such a prompt and thorough feedback. I'll start working on your suggestions today. Thanks also for your PR. As you may have noticed, English is not my mother tongue. Although I made an effort to revise the text, some errors ended up passing. |
Dear Editors and Authors, I have finished my review. Congratulations on this great work! Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Estimated hours spent reviewing:
Review CommentsIt was a pleasure to evaluate the MCTQ R package, authored by Vartanian, Benedito-Silva and Pedrazzoli. At first, It sounds interesting to use a specific R package to perform what seems to be a simple task - to process the results from a single questionnaire. However, almost nothing is simple when it comes to chronobiology, particularly because our research field deals with variables in the time domain, both using clock time and durations. This package is particularly useful to perform computations using time/date variables, to be able to read/write and derivate variables in the time domain and, also importantly, to make sure that all the proper conversion of time/date is performed. |
Thank you very much for your feedback @leocadio-miguel! I already added both @leocadio-miguel and @jonkeane as reviewers to the package documentation (changes implemented in ropensci/mctq@0536998 and ropensci/mctq@5fa5233) (see also https://gipso.github.io/mctq/authors.html). Thanks again for your excellent reviews! I'm still working on the suggestions. You can track my progress by looking at the commit history. I believe that I can finish this work by the end of the month. |
Thanks @jonkeane and @leocadio-miguel for your reviews! Thanks @danielvartan for letting us know your timeline. |
Hi @jooolia, @jonkeane, and @leocadio-miguel. Some major tasks recently knocked on my door and I couldn't take the time needed to address all the reviewers' suggestions. I would like to know if this submission can be put on hold for now. I already worked on most of the suggestions, but I would like to take some time to review all the changes before I submit them here. At the moment my schedule is pretty much full, I believe I will have some time to finish this in the first week of August when I will have some spare time. I'm sorry for the delay. I will try my best to submit all answers here ASAP. |
Hi @danielvartan , |
I would like to apologize for the delay in responding to reviewers' comments. I had some urgent tasks to handle and I wanted to take some time to be able to devote myself to the suggestions pointed out before submitting my answer. I also would like to thank the editor @jooolia and both referees @jonkeane and @leocadio-miguel for dedicating their time to reviewing our package. I have tried my best to incorporate all suggestions raised. Below I provide responses with the corresponding commit links. In response to suggestions made by @jonkeane
Response: Thank you very much @jonkeane. I have tried my best to incorporate almost all of your suggestions.
Response: I was pleased to add you as a reviewer. Changes implemented in ropensci/mctq@5fa5233.
Response: Agreed. That's the result of some bad practices I'm still trying to quit. I already made the changes you suggested.
Response: I tried to change some of the variable names, but, as you said, some of these are inevitable.
Response: Agreed. The
Response: Thanks for noticing! 🙂
Response: I already approved the PR. Thanks again! Changes implemented in ropensci/mctq@f4f9f57.
Response: Agreed. Changes implemented in ropensci/mctq@d99b734.
Response: I find that is better to be a little redundant in this case. Some utility functions may need some maturing before I consider them as
Response: I wasn't able to reproduce this warning while running
Response: Tough call, but you're right. That's a too dangerous masking. I decided to change the name to avoid the name collision you mentioned. This function is now named
Response: Sorry for that. I already deleted the
Response: Agreed. Changes implemented in ropensci/mctq@ee974cf.
Response: You're right (🤦️). Changes implemented in ropensci/mctq@5e38ef9 and ropensci/mctq@a3343e9.
Response: Agreed. In the end, I decided to remove the startup message (too much pollution). Changes implemented in ropensci/mctq@72fd8b5.
Response: Agreed. Changes implemented in ropensci/mctq@f0f9aaa.
Response: This is better understood with an example. Say you have a vector of time values that cross the midnight hour (e.g., a To make it easier, I will use a vector with the same characteristics mentioned above that can be found in the library(mctq)
data <- mctq::std_mctq$bt_w
head(data, 10)
#> 22:30:00
#> 22:50:00
#> 21:25:00
#> 00:30:00
#> 22:25:00
#> 21:00:00
#> NA
#> 22:30:00
#> 22:00:00
#> 23:00:00 If you plot a histogram with this time vector, it would look like this: library(ggplot2)
ggplot2::qplot(data, xlab = "Time", ylab = "Frequency", bins = 30) Note that the chart above uses a linear time frame, creating a gap between the data. In some cases, we need to fit the data into a circular time frame of 24h using those charts/representations. That's what This function attaches the time values to a two-day timeline by using the midday hour as a cutting point. If the time value has 12h or more, dplyr::tibble(before = head(data, 10),
after = head(mctq:::midday_change(data), 10))
#> # A tibble: 10 x 2
#> before after
#> <time> <dttm>
#> 1 22:30 1970-01-01 22:30:00
#> 2 22:50 1970-01-01 22:50:00
#> 3 21:25 1970-01-01 21:25:00
#> 4 00:30 1970-01-02 00:30:00
#> 5 22:25 1970-01-01 22:25:00
#> 6 21:00 1970-01-01 21:00:00
#> 7 NA NA
#> 8 22:30 1970-01-01 22:30:00
#> 9 22:00 1970-01-01 22:00:00
#> 10 23:00 1970-01-01 23:00:00 Now, if instead of using ggplot2::qplot(mctq:::midday_change(data), xlab = "Time", ylab = "Frequency", bins = 30) We can also use You can think of
Response: Agreed. I implemented the match for the
Response: My mistake. Changes implemented in ropensci/mctq@fbaf698.
Response: You're right (🤦). Changes implemented in ropensci/mctq@ac55804.
Response: Agreed. Changes implemented in ropensci/mctq@ae4f3a9.
Response: Agreed. Changes implemented in ropensci/mctq@3c563ef and ropensci/mctq@367cef0.
Response: You're right. I removed the
Response:
Response: Agreed. I removed some of the functionality of this function. Jenny Brian and Gábor Csárdi are working in implementing an alternative to
Response: Agreed. I split that function in two: one called
Response: I prefer to use template files instead of using the
Response: I used
Response: Agreed. The
Response: This is a tough one. On one hand, when designing R packages, you usually consider that the user has at least some knowledge about the R programming language, so you don't need to "reinvent the wheel" for every application. On the other hand, you must design a package that is easy to use by your target public.
However, after much thought and consideration, I believe that the
Response: Agreed. I decided to remove the startup message (too much pollution). Changes implemented in ropensci/mctq@72fd8b5.
Response: This is a workaround to assert that
Response: I see what you mean, but this kind of customization would require changing all of the
Response: Agreed (🤦). Changes implemented in ropensci/mctq@17a6d9e.
Response: I think that using
Response: I don't think this is a good workaround. By using
Response: About the environment, since the
Response: Agreed. Changes implemented in ropensci/mctq@dd91589 and ropensci/mctq@4ca993d.
Response: My mistake (🤦). I renamed it to
Response: Agreed. The 'class' argument wasn't really necessary. It was removed. Changes implemented in ropensci/mctq@7fac8b2.
Response: I thought of that, but, since I don't own those temporal classes, that may create issues in the long run. I feel it's best to follow Hadley Wickham's suggestion on this matter.
Response: I didn't understand what you meant here. I think you are referring to
Response: Thanks! 🙂
Response: Agree. I removed this wrapper function. Changes implemented in ropensci/mctq@d54c3bf.
Response: I'm very grateful for your review. It made me a better programmer. Thanks again! |
I'm happy to share that an abstract about In one of my talks, one of the LASC organizers recognized the lack of discussions about the topic and even suggested the inclusion of a workshop about open science in the next LASC symposium 🎉. |
very cool @danielvartan about the discussions spurred by your package and presentations. :) |
Hello @jonkeane, |
Hello, Thank you for the very very detailed responses. I've read through all of them and all of the changes or explanations are great. There were two questions (though let me know if I'm missing others!): re: goodpractice::gp() reports I've re-run it again, and that error has gone away (either I've resolved my tex dependencies, or the docs causing them have changed), either way this is resolved! re: Documentation simplification I don't see any of the repeated chunks that I remember seeing the first time — so either I imagined those or they were cleaned up already, but we can consider that resolved. In looking through, I see that there has been a lot more added to the docs, and all of that looks really great + helpful for understanding how to use {mctq}. Great job! One last comment/suggestion (though you absolutely don't need to do this, just a possible work around if you wanted):
Have you considered using Thanks both for this great package as well as the very detailed responses and explanations. |
Hi @jonkeane , About the The |
@ropensci-review-bot approve mctq |
Approved! Thanks @danielvartan for submitting and @leocadio-miguel, @jonkeane for your reviews! 😁 To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @stefaniebutland in your reply. She will get in touch about timing and can answer any questions. We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved. Last but not least, you can volunteer as a reviewer via filling a short form. |
Thanks @danielvartan all looks good to me. Thanks again for the nice package and also the nicely documented response to the reviewers' comments. It was a pleasure to look through the package and to participate in this process. I am working on getting the team ready for the transfer of the repo and you will get an invite when it is ready. Thanks! Julia |
Hi @danielvartan the invitation has been sent for the team in Github. Let me know if you don't receive it and/or if you have any issues. |
Thank you @jooolia, @jonkeane and @leocadio-miguel for all your attention and dedication in reviewing our package. I learned a lot during this process and was very happy to be a part of and contribute to the rOpenSci initiative! I already have a few other package projects underway and will soon be submitting them to the same review process. I've already transferred the package to rOpenSci and I'm now taking care of the final details. I will soon post the checklist here for you to follow this process. |
All set. Thanks again! To-dos:
|
Date accepted: 2021-10-29
Due date for @leocadio-miguel: 2021-05-26Submitting Author Name: Daniel Vartanian
Submitting Author Github Handle: @danielvartan
Other Package Authors Github handles: (comma separated, delete if none) @AnaAmeliaBeneditoSIlva, @pedrazzo
Repository: https://github.com/gipso/mctq
Version submitted: 0.0.0.9000
Editor: @jooolia
Reviewers: @leocadio-miguel, @jonkeane
Due date for @jonkeane: 2021-05-27
Archive: TBD
Version accepted: TBD
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
The Munich Chronotype Questionnaire (MCTQ) may look like a simple questionnaire, but it requires a lot of date/time manipulation. This can be really challenging, especially if you’re dealing with a large set of data.
mctq
provides reliable tools, thoroughly tested, to help scientists with that task.mctq
was designed for researchers in the field of sleep and chronobiology. The scientific applications of the package are the same as the scale it addresses, i.e. to assess peoples' sleep behavior and characteristics of circadian rhythms.No other packages do the same thing (to my knowledge).
I don't think this is applicable to our package.
N/A
Technical checks
Confirm each of the following by checking the box.
This package:
Publication options
Do you intend for this package to go on CRAN?
Do you intend for this package to go on Bioconductor?
Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: