search

jamovi / jmv

jamovi for R
https://www.jamovi.org
55 stars 27 forks source link

CFA: catch unclear error message #373

Closed raviselker closed 1 year ago

raviselker commented 1 year ago

Error message indicates that model did not converge, but gives a non-comprehensible message instead. This commit, introduces a way to catch the error message, and throw a more user-friendly error message instead.

Please give this one a thorough review @jonathon-love because I do introduce some new way of error handling here.

Fixes jamovi/jamovi#1204 Fixes jamovi/jamovi#1292

jonathon-love commented 1 year ago

should i be concerned about 'all checks have failed'?

raviselker commented 1 year ago

should i be concerned about 'all checks have failed'?

Concerned no, but you should definitely not merge this PR (and other PRs for that matter) before they succeed so I'll take a look at what's happening here.

shun2wang commented 1 year ago

maybe tests failures related to the new version of lavaan(0.6-13)? Because lavaan has recently updated and some changes may related to the calculation results .

see: bug fixes of the lavaan change logs

raviselker commented 1 year ago

maybe tests failures related to the new version of lavaan(0.6-13)? Because lavaan has recently updated and some changes may related to the calculation results .

see: bug fixes of the lavaan change logs

Only the test added in this PR failed. There were some non-ASCII characters in an error message comparison that didn't work in the test runner (but they did work locally). So I changed the error message into a partial match instead of a full match. Checks now all succeed