fatmasevinck / enetLTS

Robust and sparse methods for high diemnsional data including gaussian, binomial and multinomial families
https://github.com/fatmasevinck/enetLTS
GNU General Public License v3.0
2 stars 3 forks source link

Long runtime + minor comments (See openjournals/joss-reviews#4773) #14

Closed marastadler closed 1 year ago

marastadler commented 2 years ago

Hi @fatmasevinck,

I'm in the process of reviewing See openjournals/joss-reviews#4773. Here are some comments on the code examples:

fatmasevinck commented 2 years ago

Hi @marastadler,

Thanks for your comments, I am working on them.

fatmasevinck commented 2 years ago

Hi @marastadler

Thank you for your comments. I have done your all suggestions.

marastadler commented 2 years ago

Hi @fatmasevinck,

Thanks for addressing the comments. After removing ">" characters from the sample code, the output of your code shouldn't be part of the code chunks. You can easily solve this by adding eval = TRUE to the chunk.

Please also consider addressing the comments that I've made in the review thread regarding (typos, citation, plots, community guidelines and the chapter "related software")

fabian-s commented 2 years ago

@fatmasevinck please adress outstanding issues very soon, this has been under review for a very long time now.

as a courtesy to reviewers and me, please provide explicit links to the commits meant to fix specific issues and/or write informative commit messages so we know where to look for what.

fatmasevinck commented 2 years ago

Dear @marastadler

Many thanks for your suggestions and comments. I am working on them but there is something I could not understand which is that "After removing ">" characters from the sample code, the output of your code shouldn't be part of the code chunks. You can easily solve this by adding eval = TRUE to the chunk." Here this R lines are only for having R format. When I added eval=TRUE to the chunk, style is broken. Maybe I missed something. If so, please could you give me more info what do you mean / is it ok in this way. Thanks again,

Sevinç

fatmasevinck commented 2 years ago

Dear @marastadler

I think I have done everything you have suggested, except for the last comment. If I have missed something, your answer is very welcomed.

Thanks, Sevinç

marastadler commented 2 years ago

Dear @marastadler

Many thanks for your suggestions and comments. I am working on them but there is something I could not understand which is that "After removing ">" characters from the sample code, the output of your code shouldn't be part of the code chunks. You can easily solve this by adding eval = TRUE to the chunk." Here this R lines are only for having R format. When I added eval=TRUE to the chunk, style is broken. Maybe I missed something. If so, please could you give me more info what do you mean / is it ok in this way. Thanks again,

Sevinç

I see, so you shouldn't edit the .md-file via the console here on GitHub. You have to open this file as a .Rmd-file in R and render it to a GitHub document.

This is done as follows:

title: "..." output: github_document

Once you push your repository it will appear in the correct format.

marastadler commented 2 years ago

Dear @marastadler

I think I have done everything you have suggested, except for the last comment. If I have missed something, your answer is very welcomed.

Thanks, Sevinç

Dear @fatmasevinck,

Now I can see that you've fixed citations & typos in the .md file. But when downloading the article proof there are no updates. @fabian-s is it supposed to be like that?

From what I can see my other comments on community guidelines, coefficient plots and the chapter "related software" have not been addressed yet.

fabian-s commented 2 years ago

Now I can see that you've fixed citations & typos in the .md file. But when downloading the article proof there are no updates. @fabian-s is it supposed to be like that?

are you referring to the README.md file of the repo or the JOSS paper that gets generated from paper/paper.md in this repo?
The PDF for the latter needs to be re-generated from the updated paper.md file first, you can do that in the original review thread by running @editorialbot generate pdf -- i did just now, this should point to the latest version of the paper: https://github.com/openjournals/joss-reviews/issues/4773#issuecomment-1298246807

fatmasevinck commented 2 years ago

Dear @marastadler I think I have done everything you have suggested, except for the last comment. If I have missed something, your answer is very welcomed. Thanks, Sevinç

Dear @fatmasevinck,

Now I can see that you've fixed citations & typos in the .md file. But when downloading the article proof there are no updates. @fabian-s is it supposed to be like that?

From what I can see my other comments on community guidelines, coefficient plots and the chapter "related software" have not been addressed yet.

Dear @marastadler

It seems you are talking about README.md file. I have written "related software" section in paper.md Is that ok in this way?

fatmasevinck commented 2 years ago

@fabian-s we are on the same point at the same time, what a coincidence! Hope that this is ok for @marastadler

Many thanks, Sevinç

marastadler commented 2 years ago

@fabian-s thanks for generating the pdf. I've been referring to paper/paper.md :-)

marastadler commented 2 years ago

@fatmasevinck all I'm saying is that my comment regarding the chapter "related software" has not been addressed so far.

Here's again the comment that I made: In the chapter "related software" you name a few packages. It would be useful to elaborate a bit more on how these packages differ from enetLTS (e.g. would the package pense in the linear model case give the same results as enetLTS, or are they different and, if so, why?)

fabian-s commented 2 years ago

It would be useful to elaborate a bit more on how these packages differ from enetLTS (e.g. would the package pense in the linear model case give the same results as enetLTS, or are they different and, if so, why?)

seconded

fatmasevinck commented 2 years ago

Dear @marastadler

I added some sentences to the "related software" section.

Thanks for your comments.

fatmasevinck commented 2 years ago

Dear @fabian-s

Is there something that I should work on it? I finished everything and sent my answers before.

Many thanks for your kind effort. Sevinç

fabian-s commented 1 year ago

@marastadler have all your concerns been adressed in these recent commits?

@fatmasevinck once again, you would make this much easier for everybody if your replies/responses would specify which specific commits you intended to fix which specific issues.

marastadler commented 1 year ago

@fabian-s my comments were not or only superficially addressed yet. Here's again a summary of what's missing:

fabian-s commented 1 year ago

@fatmasevinck
please open a separate issue for each of these unresolved points and reference them in the commit messages of the commits you make to address them

fatmasevinck commented 1 year ago

I have done this.