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

Column type conversions #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kucharssim
Copy link
Member

Goes in tandem with: jasp-stats/jaspBase#136

Instead of relying on rbridge finding the readDatasetToEndNative in jaspTools, we just rely on the new data set functionality in jaspBase. I added jaspBase as a dependency explicitly now (even though we used to call it from jaspTools before as well) - or is there a reason for not doing it?

@@ -19,6 +19,9 @@ Imports:
stringi,
stringr,
testthat (>= 3.0.3),
vdiffr (>= 1.0.2)
vdiffr (>= 1.0.2),
jaspBase
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Originally, the idea was that jaspTools could do things, like installing jaspBase, for a developer. But this kinda looks like it becomes a cycle. Or is this intended as a workaround until we get rid of jaspTools altogether?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even though we already had things like jaspBase:::.vdf before, this change will make it impossible to install jaspTools without first installing jaspBase.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's not necessary, but since we are using jaspBase anyway I figured it might as well be specified as a dependency - hence I asked before whether it's deliberate that it's excluded.

So I guess I am saying I can remove it - as you say jaspTools sets up jaspBase anyway so it's not a gib deal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops sorry I missed that in your message.

@vandenman
Copy link
Contributor

** byte-compile and prepare package for lazy loading
  Error in hashtab(type = "address", NULL) : 
    could not find function "hashtab"
  Error: Error: unable to load R code in package 'jaspBase'

I'll update the R version for the unit tests, hashtab was introduced later.

@vandenman
Copy link
Contributor

@Kucharssim can you rebase/ retrigger the tests? If I rerun them they still use the old R version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants