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

JOSS Review from me [done] #2

Closed
Pentaonia opened this issue Aug 10, 2023 · 5 comments
Closed

JOSS Review from me [done] #2

Pentaonia opened this issue Aug 10, 2023 · 5 comments

Comments

@Pentaonia
Copy link

Pentaonia commented Aug 10, 2023

Review link: openjournals/joss-reviews#5753 (hopefully its now linked correctly)

Missing:

  • Writing Contributor Guidelines + maybe an issue template
  • You mention that the output is compatible with other R packages. This has been mentioned for ggplot2 and sf. I think it would be a helpful addition to include at least one or two functions from the other packages as well. That way newbies would have a starting point.
  • A link to the "reference" page that points to for example adespatial regarding the MEM/AEM maps.
  • Indicate where the 'real world example' comes from. Whether it is part of a real analysis (might a citation be appropriate?) or a "fictional" example of what can be done.

Hints:

  • It might be a good idea to include an extra chunk in the "Getting Started" vignette where you load additional packages (like ggplot2, sf). As you did for the visualisation tools page. In a vanilla R environment, at least I had to load the "ggplot2" package in addition.
  • Maybe you can adapt the functions on the "Chess pieces" page to work with the data created/used on the "Getting Started" page or the one from the paper.

Question:

  • Since you use functions from the tidyverse, why don't you work with tibbles? Because of compatibility reasons?
  • Have you checked the plots on other devices? Idk if its my fault, but it looks slightly different than in your visualization vignette. In my case, it is not a perfect circle around the focus node and it is slightly offset. It is not that dramatic but I wanted to mention it (in this step):
gg_chessboard(nodes) +
  geom_node(nodes, focus = "2-3")

image

Manuscript

  • Lines 9/10, suggestion: "It can handle directed (asymmetric) and undirected (symmetric) spatial (or non-spatial) network connections."
  • Lines 29/30: Might be the same case as for lines 9/10.

Thumbs up:

  • Great package! I would definitely incorporate it into my lecture.
  • I really like the 'reference' page in your vignette.
@Pentaonia Pentaonia changed the title Review from me [not finished yet] JOSS Review from me [not finished yet] Aug 10, 2023
@Pentaonia Pentaonia changed the title JOSS Review from me [not finished yet] JOSS Review from me [done] Aug 12, 2023
@ahasverus
Copy link
Member

ahasverus commented Aug 13, 2023

Hi @Pentaonia 👋

Thank you for reviewing this paper and the associated repository.

Here is my answers to your different points:

  1. Missing
  • Adding Contributor Guidelines

Thanks for the suggestion! Done in #4. The markdown file can be found here.

  • Adding an Issue template

Thanks for the suggestion! Done in #3. I have added three templates: one for reporting bugs, one for requesting features, and one for other questions.

  • Adding examples to link to other packages

Some examples on how to link chessboard to ggplot2 and sf are already included in the Get started vignette, especially at the end of section edge list. Do you think is enough or do you had something different in mind?

  • Adding links to reference pages of MEM/AEM

Thanks for the suggestion! Done in #5.

  • Real world example

You are right! Data used in the Get started vignette are fictional but inspired from a real study (not published yet). As these data are fictional maybe I could use fictional example instead of real-world example. What do you think?

  1. Hints
  • Loading additional packages

Done in #6. Thanks for the suggestion!

  • Using data of "Get started" in "Chess pieces" vignette

The objective of the Chess pieces vignette is to illustrate the use of the different arguments of the functions fool(), pawn(), rook(), etc. To do that I needed a square sampling design (in that case 9 transects x 9 quadrats). I decided to use a different sampling design in the Get started vignette just to illustrate the fact that chessboard works with a non-square sampling design. For the manuscript I decided to use a smaller square sampling design for visibility reasons and to provide a third example. Do you think this will confuse the user?

  1. Questions
  • Why don't you work with tibbles?

There is no particular reason why I use data.frame instead of tibble. It's just a matter of habit and also to usually limit external dependencies (which is not a good reason here ;)).

  • Plot rendering

I have tested the package on different OS and I never saw this kind of rendering issues. What OS are you using? Do you use RStudio? Is the screenshot coming from RStudio or from an export as PNG/JPG? I will investigate this issue but to me is not link to chessboard.

  1. Manuscript
  • Suggestion for lines 9/10 and lines 29/30

Done in #7. I also fixed these typos in the comment of the chunk at lines 49/50 of the manuscript, and in the README, the DESCRIPTION file, and in the Get started vignette.

Thank again for your review and contribution!

@Pentaonia
Copy link
Author

Pentaonia commented Aug 14, 2023

Hi @ahasverus 🖖

Thank you for your answers!
I just took a glimpse into your modified documents: looks great! Here my answers to the remaining questions:

  1. Missing
  • Adding examples to link to other packages

Some examples on how to link chessboard to ggplot2 and sf are already included in the Get started vignette, especially at the end of section edge list. Do you think is enough or do you had something different in mind?

So I had in mind that you might also include one or two functions from the other packages you mention in the paper. So that the end user clearly knows how to import your edge list/connectivity matrix (or maybe another data.frame/matrix) into spdep or igraph. I mean, you might have a clear idea of where the links are and how to build them. Someone else who is not so deeply involved in the field might not see the links directly and especially not the corresponding functions.

  • Real world example

You are right! Data used in the Get started vignette are fictional but inspired from a real study (not published yet). As these data are fictional maybe I could use fictional example instead of real-world example. What do you think?

Yeah it would be a bit clearer, or something like this: "Fictional scenario with real-world parallels". (But it's late now so this suggestion might be nonsense.)

  1. Hints
  • Using data of "Get started" in "Chess pieces" vignette

The objective of the Chess pieces vignette is to illustrate the use of the different arguments of the functions fool(), pawn(), rook(), etc. To do that I needed a square sampling design (in that case 9 transects x 9 quadrats). I decided to use a different sampling design in the Get started vignette just to illustrate the fact that chessboard works with a non-square sampling design. For the manuscript I decided to use a smaller square sampling design for visibility reasons and to provide a third example. Do you think this will confuse the user?

Puh, that's a good question. I would say that it is a matter of judgement: Between a larger grid of 9x9 instead of 5x5 in the paper or the possibility of not having to introduce anything new in the chess pieces vignette to be able to include the functions in an example.
If you want to leave it as it is in the paper, which is no problem, then (1) maybe refer to it and "expand" the grid from the paper to explain the functions or (2) introduce it as a new data set/grid in the chess pieces vignette. This might introduce something kind of new again, but I would assume that the reader has no problem with a new data set being used on a new page of the vignette. Especially if it is as simple as a grid, without much spatial information.

  1. Questions
  • Why don't you work with tibbles?

There is no particular reason why I use data.frame instead of tibble. It's just a matter of habit and also to usually limit external dependencies (which is not a good reason here ;)).

I was just curious because you use a handful of functions from the tidyverse, but yeah, never mind! No reason to criticise anything here! ;)

  • Plot rendering

I have tested the package on different OS and I never saw this kind of rendering issues. What OS are you using? Do you use RStudio? Is the screenshot coming from RStudio or from an export as PNG/JPG? I will investigate this issue but to me is not link to chessboard.

I'm using Windows. I have checked in all three environments (Vanilla R, R Studio and my always used VS Code whats opening the x11() plot window). I am using R version 4.3.1. Export as PDF works as expected, but as jpg/png, same as above... but then it seems like a "me problem"! ;)

All the best!

@ahasverus
Copy link
Member

Hi @Pentaonia ,

I'm just back from vacation. Thank you for your answers! Here is my answer to the remaining points:

  • Adding examples to link to other packages

So I had in mind that you might also include one or two functions from the other packages you mention in the paper. So that the end user clearly knows how to import your edge list/connectivity matrix (or maybe another data.frame/matrix) into spdep or igraph. I mean, you might have a clear idea of where the links are and how to build them. Someone else who is not so deeply involved in the field might not see the links directly and especially not the corresponding functions.

I have created a new issue (#9) that will show small examples to extend chessboard w/ different packages. I agree w/ you this can be a very good added value.

  • Real world example

Yeah it would be a bit clearer, or something like this: "Fictional scenario with real-world parallels". (But it's late now so this suggestion might be nonsense.)

I agree maybe it's too late for this suggestion.

  • Using data of "Get started" in "Chess pieces" vignette

Puh, that's a good question. I would say that it is a matter of judgement: Between a larger grid of 9x9 instead of 5x5 in the paper or the possibility of not having to introduce anything new in the chess pieces vignette to be able to include the functions in an example. If you want to leave it as it is in the paper, which is no problem, then (1) maybe refer to it and "expand" the grid from the paper to explain the functions or (2) introduce it as a new data set/grid in the chess pieces vignette. This might introduce something kind of new again, but I would assume that the reader has no problem with a new data set being used on a new page of the vignette. Especially if it is as simple as a grid, without much spatial information.

In my opinion, the user/reader can easily understand these two different datasets without being lost. But I agree and I will add a sentence in the Chess pieces vignette indicating that the network used in this vignette is derived from the one used in the paper.

Again: thanks for your contribution.

@ahasverus
Copy link
Member

I think I have answered to all your comments.
All the bests!

@Pentaonia
Copy link
Author

Hey @ahasverus,

looks great to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants