dpc10ster / RJafroc

Artificial Intelligence: Evaluating AI, optimizing AI
19 stars 8 forks source link

Cran3 #68

Closed dpc10ster closed 3 years ago

dpc10ster commented 3 years ago

I have updated the cran-comments.md file on cran3 branch; can you review?

codecov[bot] commented 3 years ago

Codecov Report

Merging #68 (a1f5276) into master (03178f3) will decrease coverage by 73.65%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #68       +/-   ##
==========================================
- Coverage   73.66%   0.01%   -73.66%     
==========================================
  Files          81      81               
  Lines        9820    9820               
==========================================
- Hits         7234       1     -7233     
- Misses       2586    9819     +7233     
Impacted Files Coverage Δ
R/Compare3ProperRocFits.R 0.00% <ø> (-31.62%) :arrow_down:
R/DfBinDataset.R 0.00% <ø> (-84.46%) :arrow_down:
R/DfReadCrossedModalities.R 0.00% <ø> (-70.00%) :arrow_down:
R/DfReadDataFile.R 0.00% <ø> (-89.27%) :arrow_down:
R/DfReadLrocDataFile.R 0.00% <ø> (-98.56%) :arrow_down:
R/FitRsmRoc.R 0.00% <ø> (-85.81%) :arrow_down:
R/PlotRsmOperatingCharacteristics.R 0.00% <ø> (-46.31%) :arrow_down:
R/SimulateFrocDataset.R 0.00% <ø> (-97.57%) :arrow_down:
R/SimulateFrocFromLrocDataset.R 0.00% <ø> (ø)
R/SimulateLrocDataset.R 0.00% <ø> (ø)
... and 82 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 03178f3...9de3f95. Read the comment docs.

pwep commented 3 years ago

There is a merge conflict for this pull request on the following files:

tests/testthat/test-PlotEmpiricalOperatingCharacteristics.R
tests/testthat/test-predicted-plots.R

cran3 is missing the tests directory. The test files were removed in commit 83e4116c60fccbb9151a8695e0345da472e0c04c on the cran3 branch. Merging with developer branch (in commit aed6d3ebbfabce474f25e96156c0245aa9d47ebf) did not add them back in.

Should the test directory files be there for the merge into master?

pwep commented 3 years ago

From cran-comments.md

17. "ubuntu-rchk", Ubuntu Linux 16.04 LTS, R-devel with rchk:
    - ERROR too many states (abstraction error?) in function strptime_internal

This looks to be an error with the way rchk modules certain functions. I will look to isolate the error by running the ubuntu-rchk image locally.

dpc10ster commented 3 years ago

Thanks; someday you have teach me about docker files and how to run these checks on your local computer;

If this is not on the CRAN check listt, we can move on. (I dont see it.)

Dev

On Dec 10, 2020, at 8:36 PM, Peter Phillips notifications@github.com wrote:

From cran-comments.md

  1. "ubuntu-rchk", Ubuntu Linux 16.04 LTS, R-devel with rchk:
    • ERROR too many states (abstraction error?) in function strptime_internal This looks to be an error with the way rchk modules certain functions. I will look to isolate the error by running the ubuntu-rchk image locally.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/68#issuecomment-742908044, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4NJRCRGYFQC72XHSE3E7LSUFZSBANCNFSM4UVS2RUQ.

dpc10ster commented 3 years ago

Since all tests have been removed from cran3, should we just delete the coverage yaml file? That would remove the check failures, right?

dpc10ster commented 3 years ago
pwep commented 3 years ago

To resolve the merge conflict, I will delete the following files in master:

tests/testthat/test-PlotEmpiricalOperatingCharacteristics.R
tests/testthat/test-predicted-plots.R

These two files would have been deleted anyway.

dpc10ster commented 3 years ago

Do I really want to merge cran3 (the stripped down version) into master? I was just going to use your comments on cran3 to make the CRAN submission. Can you just tell me your changes on the cran3 branch so I can incorporate them into my submission.

dpc10ster commented 3 years ago

I got your changes on cran3 branch. Plan to submit Monday. If you have any more changes ...

dpc10ster commented 3 years ago

I don't understand why I am missing the two files you made the fixes on with check.environment = FALSE in the master branch, but they are present in the developer branch, but when I try to merge from developer it says it is up to date with master?

pwep commented 3 years ago

It is a good question. Looking in the network diagram at https://github.com/dpc10ster/RJafroc/network a few things have happened. I've added a screenshot below.

The developer branch (in blue) was merged into master (black) and cran3 (gold) at point 1 on the image below.

At point 2 my pull request (green) adding check.environment = FALSE was merged into developer and then into master. It did not make it into cran3, and I think in the meantime cran3 deletes these files. These gives the merge error in this pull request, which is trying to merge cran3 into master.

Screenshot 2020-12-12 at 01 06 03

they are present in the developer branch, but when I try to merge from developer it says it is up to date with master?

Nothing new has been changed in the developer branch since the last merge into master (point 2 on the network above), so it is "up to date".

pwep commented 3 years ago

It is all one-way: developer -> master -> cran3

So this pull request is going in the wrong direction? It is trying to merge from cran3 into master.

Looking again at your comment above....

Do I really want to merge cran3 (the stripped down version) into master?

I guess the answer is no, so it might be an idea to close this pull request without the merge into master. I've been trying to honour the intention of the pull request (hence deleting those files from master to allow the merge).

dpc10ster commented 3 years ago

Thanks for the insight; I am relieved. Any suggestions on improved workflow, for future submits, are welcome; I will wait till Monday to submit to CRAN. Thanks!

On Dec 11, 2020, at 8:21 PM, Peter Phillips notifications@github.com wrote:

It is a good question. Looking in the network diagram at https://github.com/dpc10ster/RJafroc/network https://github.com/dpc10ster/RJafroc/network a few things have happened. I've added a screenshot below.

The developer branch (in blue) was merged into master (black) and cran3 (gold) at point 1 on the image below.

At point 2 my pull request (green) adding check.environment = FALSE was merged into developer and then into master. It did not make it into cran3, and I think in the meantime cran3 deletes these files. These gives the merge error in this pull request, which is trying to merge cran3 into master.

https://user-images.githubusercontent.com/732331/101968357-b5aea200-3c16-11eb-9db6-a0cd13523dbc.png they are present in the developer branch, but when I try to merge from developer it says it is up to date with master?

Nothing new has been changed in the developer branch since the last merge into master (point 2 on the network above), so it is to to date.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/68#issuecomment-743581766, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4NJRAXUBEIVODLIWUIKK3SULAQRANCNFSM4UVS2RUQ.

dpc10ster commented 3 years ago

Will wait till Monday to submit;