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

update on cran before 30/10 #469

Closed
DominiqueMakowski opened this issue Oct 5, 2021 · 63 comments
Closed

update on cran before 30/10 #469

DominiqueMakowski opened this issue Oct 5, 2021 · 63 comments
Labels
High priority 🏃 This issue should be addressed soon

Comments

@DominiqueMakowski
Copy link
Member

Dear maintainer,

Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_bayestestR.html.

Please correct before 2021-10-19 to safely retain your package on CRAN.

which is also the date of my birthday

@strengejacke
Copy link
Member

which is also the date of my birthday

🥳

@bwiernik
Copy link
Contributor

bwiernik commented Oct 6, 2021

Ain't no party like a CRAN deadline party.

@strengejacke
Copy link
Member

@mattansb @DominiqueMakowski There a few tests failing on CRAN (https://www.r-project.org/nosvn/R.check/r-devel-windows-x86_64/bayestestR-00check.html), but some more here on GitHub. Are you able to fix these in time?

@mattansb
Copy link
Member

I have a PR (#474) that I will not merge yet, as it introduces some more complications.

I don't know what the CRAN errors are about - looks like Dom's domain.

As for the error on the main branch - do you know where the error is happening? I can't figure it out from the log at https://github.com/easystats/bayestestR/runs/3895314216?check_suite_focus=true

@strengejacke
Copy link
Member

This is the current error log:
https://github.com/easystats/bayestestR/runs/3913495285

@strengejacke
Copy link
Member

Looks like some recent changes in insight that cause problems in bayestestR need to be fixed...

@strengejacke
Copy link
Member

@mattansb I'm not sure, there seem to be some issues with unupdate(): https://github.com/easystats/bayestestR/runs/3913495285#step:10:1014

@mattansb
Copy link
Member

Weird... Seems like cmdstanr needs to be added to Suggests 🤷

mattansb added a commit that referenced this issue Oct 16, 2021
@DominiqueMakowski
Copy link
Member Author

Hello 👋

need to be fixed...

you mean fixing as in commenting out the failing tests 😅?

@mattansb
Copy link
Member

The current errors seem to have something to do with BRMS? Taking too long? I'm not quite sure how to read the logs...

@strengejacke
Copy link
Member

══ Failed tests ════════════════════════════════════════════════════════════════
── Error (test-bayesfactor_parameters.R:85:7): bayesfactor_parameters BRMS ─────
Error: Error in `unupdate.brmsfit(brms_mixed_6)`: Error : Please install the 'cmdstanr' package.

Backtrace:
    █
 1. ├─bayestestR::unupdate(brms_mixed_6) test-bayesfactor_parameters.R:85:6
 2. └─bayestestR:::unupdate.brmsfit(brms_mixed_6)
── Failure (test-check_prior.R:36:3): (code run outside of `test_that()`) ──────
check_prior(model2)$Prior_Quality (`actual`) not equal to c(...) (`expected`).

    actual          | expected           
[1] "uninformative" | "uninformative" [1]
[2] "informative"   - "uninformative" [2]
[3] "informative"   | "informative"   [3]
[4] "uninformative" | "uninformative" [4]
[5] "uninformative" | "uninformative" [5]
── Failure (test-check_prior.R:44:3): (code run outside of `test_that()`) ──────
check_prior(model2, method = "lakeland")$Prior_Quality (`actual`) not equal to c(...) (`expected`).

    actual           | expected         
[1] "informative"    | "informative" [1]
[2] "misinformative" - "informative" [2]
[3] "informative"    | "informative" [3]
[4] "informative"    | "informative" [4]
[5] "informative"    | "informative" [5]
── Failure (test-describe_posterior.R:265:7): (code run outside of `test_that()`) ──
describe_posterior(ttestBF(mtcars$wt, mu = 3), ci = 0.95) (`actual`) not equal to structure(...) (`expected`).

actual vs expected
                  CI_high
- actual[1, ]   0.1557581
+ expected[1, ] 0.1792614

  `actual[[5]]`: 0.16
`expected[[5]]`: 0.18

[ FAIL 4 | WARN 23 | SKIP 3 | PASS 317 ]
Error: Error: Test failures

@mattansb
Copy link
Member

That cmdstanr error is very weird... What that model insight::download_model("brms_mixed_6") fit with cmdstanr???

Anyway, you can comment that chunk out for now (sorry, I don't have time to get to this.......)

@strengejacke
Copy link
Member

ok, @DominiqueMakowski, you may sent to win-builder and then to CRAN? Looks like all other issues have been resolved...

@strengejacke
Copy link
Member

strengejacke commented Oct 18, 2021

https://easystats.github.io/parameters/reference/ci.merMod.html is now https://easystats.github.io/parameters/reference/ci.default.html

The two failing tests are probably cmdstanr issues? I suggest skipping those on CRAN.

@strengejacke
Copy link
Member

Oh, and you need to remove the remotes field from the description file :-)

@DominiqueMakowski
Copy link
Member Author

Thanks, we see:

 URL: https://easystats.github.io/parameters/reference/ci.merMod.html
   From: man/ci.Rd
   Status: 404
   Message: Not Found

Please fix and resubmit.

@strengejacke
Copy link
Member

Thanks, we see:

 URL: https://easystats.github.io/parameters/reference/ci.merMod.html
   From: man/ci.Rd
   Status: 404
   Message: Not Found

Please fix and resubmit.

See my comment (#469 (comment)) above:
https://easystats.github.io/parameters/reference/ci.merMod.html is now https://easystats.github.io/parameters/reference/ci.default.html

DominiqueMakowski added a commit that referenced this issue Oct 19, 2021
@DominiqueMakowski
Copy link
Member Author

See my comment

Oops, sorry I am now an old man my memory and attention span are gone ^^

@strengejacke
Copy link
Member

image

DominiqueMakowski added a commit that referenced this issue Oct 20, 2021
@IndrajeetPatil
Copy link
Member

I am guessing the archival deadline was extended to 2021-11-02?

@DominiqueMakowski
Copy link
Member Author

til the 31/10, but i submitted yesterday and I am now waiting for a reply about some dead links false positives

@IndrajeetPatil
Copy link
Member

I would consider re-submitting (no need to change version number) because it can take days if not weeks before someone looks at the e-mail, and we don't want to push our luck.

@DominiqueMakowski
Copy link
Member Author

yeah you're right, but because of the "possibly dead links" message as I didn't fix them i am worried that it will get back to me again without a manual inspection

@IndrajeetPatil
Copy link
Member

If they are not too crucial, I'd say skip those URLs for now and resubmit.

@DominiqueMakowski
Copy link
Member Author

let's do that, I'll resubmit today

@DominiqueMakowski
Copy link
Member Author

cran contacted me 2 days ago after submission that it breaks some down downstream deps (see) I said we are aware and will fix asap but since then no news :/ (status is waiting in https://lockedata.github.io/cransays/articles/dashboard.html)

@strengejacke
Copy link
Member

Strange, but why did this not occur during submission? Anyway, I have revised tests, forcing them to be skipped on CRAN, that's the safest way I'd say,

@IndrajeetPatil
Copy link
Member

How did {see} update ever make it to CRAN?! This test should have failed during the submission process itself.

@IndrajeetPatil
Copy link
Member

IndrajeetPatil commented Oct 29, 2021

Strange indeed.

At any rate, the safest way to resolve this ASAP will then be:

  • update see on CRAN?
  • run revdepcheck for bayetestR again
  • mention in cran_comments if you see any problems (no problems seen)
  • resubmit bayestestR

@DominiqueMakowski
Copy link
Member Author

Rchecks work in mysterious ways

@IndrajeetPatil IndrajeetPatil changed the title update on cran before 19/10 update on cran before 30/10 Oct 29, 2021
@IndrajeetPatil
Copy link
Member

I am running revdepcheck now and will make a PR in a bit

@strengejacke
Copy link
Member

Note that updating see will completely break plotting for correlation.

@IndrajeetPatil
Copy link
Member

How does every update in one package in our ecosystem breaks something in another package?! 😭

@IndrajeetPatil
Copy link
Member

IndrajeetPatil commented Oct 29, 2021

So bizarre, I see no issue with CRAN {see} in revdepcheck. I am guessing CRAN is running theirs on some other architecture.

-- CHECK ---------------------------------------------------------------------------- 18 packages --coveffectsplot 0.0.9.1                 -- E: 0     | W: 0     | N: 1correlation 0.7.1                      -- E: 0     | W: 0     | N: 0datawizard 0.2.1                       -- E: 0     | W: 0     | N: 0effectsize 0.5                         -- E: 0     | W: 0     | N: 0emmeans 1.7.0                          -- E: 0     | W: 0     | N: 1eiCompare 3.0.0                        -- E: 0     | W: 0     | N: 0fbst 1.5                               -- E: 0     | W: 0     | N: 0multifear 0.1.2                        -- E: 0     | W: 0     | N: 1modelbased 0.7.0                       -- E: 0     | W: 0     | N: 1neatStats 1.7.3                        -- E: 0     | W: 0     | N: 0insight 0.14.5                         -- E: 0     | W: 0     | N: 0performance 0.8.0                      -- E: 0     | W: 0     | N: 0psycho 0.6.1                           -- E: 0     | W: 0     | N: 0report 0.4.0                           -- E: 0     | W: 0     | N: 0parameters 0.15.0                      -- E: 0     | W: 0     | N: 0sjstats 0.18.1                         -- E: 0     | W: 0     | N: 0see 0.6.8                              -- E: 0     | W: 0     | N: 0sjPlot 2.8.9                           -- E: 0     | W: 0     | N: 0                                                                  
OK: 18                                                                                            
BROKEN: 0

@strengejacke
Copy link
Member

strengejacke commented Oct 29, 2021

How does every update in one package in our ecosystem breaks something in another package?! 😭

This is due to the change in the underlying plotting API, namely the switch to visualization_recipe(). While this is a clever change, it's a big breaking change, so this stuf is expected and will hopefully be resolved in the next release cycle(s).

Or in other words: @DominiqueMakowski managed it to introduce ecosystem breaks in each single easystats package :-D

@IndrajeetPatil
Copy link
Member

Dom giveth, and Dom taketh away...

@IndrajeetPatil
Copy link
Member

I think we should ask for another extension for submission. This is all not going to be resolved by tomorrow.

What do you think?

@IndrajeetPatil IndrajeetPatil added the High priority 🏃 This issue should be addressed soon label Oct 29, 2021
@strengejacke
Copy link
Member

strengejacke commented Oct 29, 2021

We can simply submit bayestestR, because failing checks are not related to see. And then submit an update for see within the next two weeks.

@DominiqueMakowski
Copy link
Member Author

On its way to cran

@strengejacke
Copy link
Member

Interestingly, see does not yet fail on CRAN: https://cran.r-project.org/web/checks/check_results_see.html

Not sure if we will expect something else the next days?

@DominiqueMakowski
Copy link
Member Author

I don't think there will be more breaking changes related to Vizrecipes as what led to breaks was the iterative moving of the function definition and related changes, but now that's stabilized so it should be okay. I don't know why but today I feel like the new dawn of a stable easytats has started 🌄

@strengejacke
Copy link
Member

I don't think there will be more breaking changes related to Vizrecipes

No no, I was referring to the CRAN issue from revdep checks (breaking see), but still, as of today, CRAN checks for see are all ok. So the revdep-issue could have been a false positive.

@IndrajeetPatil
Copy link
Member

Maybe we can submit {modelbased} now?

@DominiqueMakowski
Copy link
Member Author

should we wait for see ?

@strengejacke
Copy link
Member

Do we need to wait for see? And I'm not sure current see works with parameters.

@DominiqueMakowski
Copy link
Member Author

well then ok i'll submit modelbased asap

@IndrajeetPatil
Copy link
Member

Any update on the submission? Is it stuck at some stage?

@DominiqueMakowski
Copy link
Member Author

stuck at the "didn't submit yet" stage ^^ but will move it to "submit now"

@strengejacke
Copy link
Member

If you're talking about modelbased, are all relevant issues resolved and PRs merged? Is there any hurry, or should we see until all relevant stuff is implemented?

@DominiqueMakowski
Copy link
Member Author

I don't think there's a hurry unless @IndrajeetPatil knows something I missed

@IndrajeetPatil
Copy link
Member

No hurry. I just wanted the ecosystem to remain in sync.
The last release for all packages was in September/October, except {modelbased}.

@DominiqueMakowski
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High priority 🏃 This issue should be addressed soon
Projects
None yet
Development

No branches or pull requests

5 participants