Column type conversions

[Kucharssim]

fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/2332

Not sure if this is really the best approach, but it does resolve the issue. I still need to check if really everything works as expected, I welcome any input.

[vandenman]

This looks really good!

However, this is a pretty aggressive change. I wonder whether we can create an opt-in way for adding this to a module, like how we once had jaspResults: true in the QML files. Because I can imagine that many hidden issues appear when this is suddenly some R package receives a jaspScale instead of a numeric and that we need to sprinkle some jasp2r calls around.

[vandenman]

/test-downstream

Automatic edit:
Running downstream tests for commit cd4614b, follow the run here.

[github-actions]

The following downstream tests for commit cd4614b failed:

Module	windows-latest	macOS-latest	ubuntu-latest
jaspAnova	❌	❌	❌
jaspDescriptives	❌	❌	❌
jaspFactor	❌	❌	❌
jaspFrequencies	❌	❌	❌
jaspRegression	❌	❌	❌
jaspTTests	❌	❌	❌

The full log is available here.

[Kucharssim]

Because I can imagine that many hidden issues appear when this is suddenly some R package receives a jaspScale instead of a numeric and that we need to sprinkle some jasp2r calls around.

I am not sure if I am overlooking a case that you have specifically in mind. But in general the code inside of modules will still run on base R types as the data is obtained using readDataSetToEnd which should return either numeric, ordinal, or factor. This whole jaspType shenanigans is used only to simulate readDataSetToEnd's behavior when run from R.

Unless of course we want to fully switch to JASP column types inside of analyses but then indeed we would need to implement this gradually because the current module code is not written for it.

[vandenman]

/test-downstream

Automatic edit:
Running downstream tests for commit 072d585, follow the run here.

[github-actions]

The following downstream tests for commit 072d585 failed:

Module	windows-latest	macOS-latest	ubuntu-latest
jaspAnova	❌	❌	❌
jaspDescriptives	❌	❌	❌
jaspFactor	❌	❌	❌
jaspFrequencies	❌	❌	❌
jaspRegression	❌	❌	❌
jaspTTests	❌	❌	❌

The full log is available here.

[vandenman]

I am not sure if I am overlooking a case that you have specifically in mind.

Well, unless the GitHub action is doing something wrong, this PR breaks all unit tests from all modules. So my use case is, do we need to change something in the modules (or jaspTools) to make this PR not break everything? If it needs to be done in each module, I would imagine we prefer to do that incrementally.

[Kucharssim]

I'll check what's wrong in the tests, the PR should definitely not break this.

[Kucharssim]

Ah, of course. I forgot that jaspTools tries to load the dataset through implementing its own readDataSetToEnd that is found by jaspBase using getAnywhere() - a connection that I broke with this PR.

So this PR will need to be accompanied by a little change to jaspTools so that the unit tests are able to load the correct dataset - I'll make a PR for that.

Anyway, I see that some of the downstream tests did not even make it to run the tests because stringi could not be installed/loaded. I think stringi is suggested by pillar which is used for some of the pretty printing of the jaspTypes in R console, but I guess I can remove that. What do you think @vandenman?

[shun2wang]

I think I once fixed this error with stringi see https://github.com/jasp-stats/INTERNAL-jasp/issues/2192, the libicu library which is dependence of R packages such as igraph on Linux I guess.

so I made: jasp-stats/jasp-actions#30

[Kucharssim]

So in combination with jasp-stats/jaspTools#38 the common modules now pass their tests on my computer. I will test the rest as well just to make sure that all is really ok.*

*Unless of course we do indeed want to make this something module maintainers can opt-in if they want syntax enabled, but I still think it would be nice to be able to switch everything everywhere all at once

[vandenman]

/test-downstream

Automatic edit:
Running downstream tests for commit 099df1b, follow the run here.

[vandenman]

🤦‍♂️ this is not using the new jaspTools so of course all the tests fail...

[github-actions]

The following downstream tests for commit 099df1b failed:

Module	windows-latest	macOS-latest	ubuntu-latest
jaspAnova	❌	❌	❌
jaspDescriptives	❌	❌	❌
jaspFactor	❌	❌	❌
jaspFrequencies	❌	❌	❌
jaspRegression	❌	❌	❌
jaspTTests	❌	❌	❌

The full log is available here.

[Kucharssim]

this is not using the new jaspTools so of course all the tests fail...

yes that's why I was running the tests manually on my computer. Can we specify a specific branch/commit of jaspTools in the downstream tests, or do you want to just merge the jaspTools PR first and then run the tests here? Note that once we merge the jaspTools PR, tests will be broken without having the corresponding changes to jaspBase introduced in this PR.

[Kucharssim]

@vandenman @JorisGoosen this is basically outdated, right? Should I close this so that it doesn't hang around here forever?

[vandenman]

Nah leave it open, I'd still like to implement this someday. Perhaps if Joris adds another option for QML to enable this we can look at doing this more gradually though?