example_scripts/ropensci_reviews_checklist.md

In vbaliga/pathviewR: Wrangle, Analyze, and Visualize Animal Movement Data

Response to reviewers for rOpenSci reviews - round 1

Hi @asbonnetlebrun, @marcosci, and @maelle,

Thank you sincerely for taking the time and effort to review our submission.

Below, you will find point-by-point responses to each of your comments. Your original comments may not appear fully (for sake of concision), but please note that we believe that we have addressed each item to the best of our capability.

Since addressing these items was handled over several commits, the most appropriate commit that reflects the change will also be noted within each of our responses.

Please also note that the package is now entitled pathviewr instead of the original pathviewR, per your advice.

Review from Reviewer 1 (asbonnetlebrun)

Documentation

Examples for all exported functions in R Help that run successfully locally

There are examples for almost all exported functions in R help. Exceptions are the calc_sf_box(), calc_sf_V(), calc_vis_angle_box() and calc_vis_angle_V() functions (and standardised_tunnel, although there is a reason given in the R help – because no example data is provided with pathviewr to run the code on).

We added in examples for each of the following functions. Please note that we consoldiated some of the visual guidance functions, so some of the original functions referred to in this reviewer comment have now been replaced or renamed.

• get_sf() (99c9ef5)
• get_vis_angle() (dfd1e2f)(6a36758)
• get_min_dist_box() (6a36758)
• get_min_dist_v() (b5f1445)

Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Only in the DESCRIPTION file, but not in the README.

We have added contribution guidelines (with links to Issues pages) in our README. (f5d47e7)

Functionality

We will address these items in the Review Comments section below

Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

We have added you as a "rev" within our description file (5b0661). Please let us know if you would like your name to appear differently and/or would like to adjust contact info etc.

Review Comments

Data import and cleaning

My understanding is that the “Relabeling axes” and “gathering data columns” parts are only relevant in the case of Motive data (Flydra data already comes in the correct format, and as_viewr(), the function to import other types of data also takes data in the correct format and already relabels the columns). Maybe this could be made a bit clearer in the vignette?

We have clarified language of this vignette to indicate that relabeling & gathering are only necessary in certain cases (e.g. using Motive data). Please let us know what you think. (591dd50)

Maybe that was me not being very familiar with the kind of experiments the package is relevant for, but I had trouble to understand when we would use the standardize_tunnel() function or the rotate_tunnel() function. Maybe you could provide some examples when these functions to be applied? Also I struggled to understand the standardize_tunnel() function, maybe particularly because there was no example in the help file. When would the landmarks already be in the data? I guess it will never be the case with Flydra data, considering that the read_flydra_mat() function allows to input only a single subject_name. Is that why it doesn’t need to be rotated, or is that something arising from the Flydra software?

We have clarified the circumstances under which standardization is needed and what types of landmarks are appropriate (591dd50) Additionally, in the Help file for standardize_tunnel(), we have clarified this function's use cases (591dd50)

Also, considering how the select_x_percent() function works (by selecting a certain percentage of the tunnel length on each site of (0,0,0) along the length axis), shouldn’t it be more appropriate to say that the (0,0,0) must be at the centre of the region of interest, rather than at the centre of the tunnel?

Thanks! We have revised language of this vignette on what (0,0,0) represents (591dd50)

Minor point: the link to the vignette for managing frame gaps is missing in the text.

Thanks for catching this -- we have added the link (591dd50)

Managing frame gaps

Very minor point: could it be clearer in the visualize_frame_gap_choice() help or in the vignette that the loops argument represents the maximum frame gap considered (i.e. that each loop represents an increment of 1 on the frame gap value)?

We have added details of what loops means to both the vignette (bcf6424) and to the Help file (bcf6424)

Could there be cases when frame gaps can vary between devices (i.e. if I understood well in the case of the Motive data, between subjects)? If so, would it be relevant to allow the autodetect approach to be applied to each subject separately? (or maybe that is not relevant...)

Sorry for being unclear - this function actually has this feature! We have made a slight modification to the language that will hopefully make it more clear (62b63b7)

Visual perception

This is where I struggled, but maybe that is because I don’t come from the specific field of application of this package. I felt like some more details could be included in the vignette. Maybe start with a few examples of experiments where this package can be useful. Maybe you could say right at the start that these functions are relevant for experiments with animals flying in tunnels with visual stimuli consisting of sine wave gratings on the tunnel walls?

Thanks. We have heavily revised the language of the first couple paragraphs accordingly, and a figure has also been inserted to hopefully give readers a better sense of the concepts at hand. Please let us know what you think! (97704bc)

Also, my understanding is that here you implement only two cases:

Motive example: birds flying through a V-shaped tunnel, with visual stimuli on each side of the tunnel Flydra example: birds flying through a rectangular tunnel, with visual stimuli on the side and front walls. So maybe make it clear that this section only applies to these two specific cases?

On this, a question: is there any plan to code more situations or are these the typical situations for these kinds of experiment? If you welcome suggestions from users of other settings, maybe you could say that in the vignette in the same way you mention that you are happy to work towards the inclusion of more data types?

We have revised the language of how these experiments are introduced (97704bc). We have also provided links to the Issues page where users can request e.g. different tunnel setups (20bb54a)

More generally, I would have appreciated some definition of what you call “spatial frequency” and “visual angle”. Although I understand that these might be obvious for people in the field (which are ultimately the target users of the package), but I feel like these terms (visual angle, and in particular spatial frequency) are of such broad usage that they might deserve some better description of what they mean in the context of the package. Maybe include a diagram in the vignette for users to visualise what these values represent? In particular, the Value section in the R help for the calc_sf_box() function seems to mention that the spatial frequency is the number of cycles per degree of visual angle. Maybe this info could be included in the vignette (and in the Description section of the function help)?

We have added in definitions along with citations of some journal articles that provide nice summaries of these topics. (fa7b12e)

Comments on the code of the visual perception functions

I seem to understand that the functions to calculate visual angle and spatial frequency in the box example don’t handle the front wall (only the side walls), am I right? Is there any reason for this? If the front wall is not relevant, maybe there is no need to include the option to add parameters for it in the insert_treatments() function?

Thanks for the suggestion. The visual perception functions now include calculations for the end walls, though the outputs from these functions only include the end wall towards which the subject is moving towards. Please see the changes to following functions:

• get_sf() (99c9ef5)
• get_vis_angle() (dfd1e2f)(6a36758)
• get_min_dist_box() (6a36758)
• get_min_dist_v() (b5f1445)

Note from VBB: for brevity, the details of your calculations will not be included here.

On that note, there is no need for an ifelse() to calculate these distances, these lines could simply replace these lines and these lines in calc_sf_box() and calc_vis_angle_box()

Thanks! We have now replaced the ifelse() lines in the corresponding functions, which also have been renamed:

• get_sf() (99c9ef5)
• get_vis_angle()(6a36758)

Can you please correct the calculations, and adapt the test-calc_vis_angle_box.R file?

Thanks sincerely for catching this! We have made corrections to the calculations of vis angle and SF (6a36758)(99c9ef5).

We have also updated test-get_vis_angle.R accordingly Tests were written for all new functions (calc_min_dist_v()(93808ea) , calc_min_dist_box(93808ea) , get_vis_angle()(ba8d813)., and get_sf())(b4afca9) .

And also on this, I noticed that the user can supply negative values for the neg_wall and pos_wall arguments in the insert_treatments() function, which in this case would lead to spurious distances being calculated. Maybe insert a warning/error message if that is the case, and add the appropriate tests?

Great catch! We have now added guidance to the Help file of insert_treatments() about feasible values for all arguments including new tunnel dimensions arguments tunnel_width and tunnel_height(429258b). This includes a check within insert_treatments() that stops the function if faulty argument values are supplied (429258b). A test to test-insert_treatments.R to check the functionality of the above check has also been added (a9cc804)

Spatial frequency

Another point, on the spatial frequency. If I understand it well (being the number of cycles per degree of visual angle), I think there is an error in these lines of calc_sf_V() and these lines of calc_sf_box().

Thanks, we have made corrections to the calculation of SF (99c9ef5)

Just one final broader question, don’t these calculations assume that the bird is parallel to the length axis of the tunnel? Is that not important/common practice not to use the rotation information?

Yes, this is true and a great point. It is actually not common practice in the field to use rotation information -- quite a few studies still rely solely on positional data. That said, we definitely agree that adding rotation is an important way to advance our understanding of visual guidance. At this time, we allow for rotation data to be imported & wrangled along with all the positional data, but rotation data have not yet been integrated into the visual perception functions. This is largely because how rotation data are encoded can be tricky (depending on how the rotation matrix is composed and/or how orientation axes are defined on each subject). Accordingly, we are still working on a way that will work generically for most use cases.

For now, we have added a note in the Help files for each visual perception that rotation information may be integrated in future pathviewr updates (46ba850). We have also done so in the Visual perception vignette (a3a7795)

Review from Reviewer 2 (marcosci)

Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

We have added you as a "rev" within our description file (5b0661). Please let us know if you would like your name to appear differently and/or would like to adjust contact info etc.

Review Comments

In the description etc. you state that pathviewr is a tool for "animal movement data" ... Couldn't you make that broader? Part of the package is actually capable to handle all kinds of 3D movement data and the visual perception function should also be applicable to humans, or?

Yes thanks, this is a good point and we agree. We have made a slight alteration to our description and readme. We don't want to oversell the features of our package, so we hope that where we have landed with the language strikes a nice balance. (deffd9d)

It would actually be quite beneficial to give a short walkthrough of what input data can look like. So, you need x,y,z ... but what more. And what defines Optitrack and flydra data.

We agree and added a short walkthrough of what movement data look like, both generally and specifically in Motive and Flydra (591dd50)

I did not see any contribution guidelines, so it would be helpful to include those.

We have added contribution guidelines provided by rOpenSci (with slight modification) (5252918)

The Maintainer field is missing in the DESCRIPTION - altough Vikram is listed as CRE.

We've updated the Maintainer field with VBB as the maintainer (9146e09)

pathviewR ... I submitted a package here once and the feedback was to either stick with capital letters or just lower case. I don't know if that changed, but it makes typing the package name out easier in my opinion. If that is a must should probably be addressed by @maelle.

Yeah, we see what you mean and went ahead and changed the name to pathviewr. (52e559c)

Additional items from the editor (maelle)

Regarding the package name, yes we recommend all lower-case, especially if the package isn’t on CRAN yet. I myself renamed my Ropenaq package after review and I don’t regret doing it.

As noted above, we changed the name to pathviewr. (52e559c). In particular, we found this guide to be very helpful -- we went step-by-step through everything in that page. We figured it would be good to let you know in case you are interested in providing that sort of info in the rOpenSci Guide for Authors, though we understand it may not be common enough of a problem to merit adding to the guide.

Thanks again for all your feedback and advice!

And sorry for the slight delay in getting back to you. My (VBB's) wife just gave birth to our first child last week -- it's been a pretty wild ride!

Best regards, Vikram 🐢

vbaliga/pathviewR documentation built on March 16, 2023, 4:13 p.m.