search

juba / questionr

R package to make surveys processing easier
https://juba.github.io/questionr/
82 stars 17 forks source link

Updating documentation of look_for() #111

Closed larmarange closed 4 years ago

larmarange commented 4 years ago

Dear @juba

the new version of labelled is on its way to CRAN. It includes changes in look_for() function (changes already mentioned in a previous PR). These changes include a new argument and a change of the default value of another argument.

That change creates an error when checking questionr package because questionr documentation is not in line anymore with the new version of the function.

A simple devtools::document() using the new version of labelled will fix the problem.

Maybe for consistency, it would also be relevant, when labelled 2.6.0 on CRAN, to update DESCRIPTION to force that version of labelled and avoid inconsistency in documentation.

An alternative is to change the current import of look_for() in questionr, removing the full documentation in questionr and simply linking to labelled. Let me know what you prefer.

Regards

juba commented 4 years ago

Hi,

I'm not really opinonated about this, but if lookfor is primarily defined in labelled, maybe the simplest would be to remove the import and transform questionr::lookfor into a deprecation error with a message saying to use labelled::lookfor ? It would avoid duplicating code and documentation.

If you're ok with this don't hesitate to submit a PR. The only obligation is to do it soon, because I just received a warning message from CRAN about questionr asking to solve this problem in particular before september 10th.

Thanks !

larmarange commented 4 years ago

See PR #112

It is a simple import/export from labelled. So the functions are still available in questionr but documentation will simply redirect to labelled, avoiding any inconsistency between the function and the documentation.

juba commented 4 years ago

Merged. You're right, that's clearly the best solution.

I'll wait for labelled to be published and push questionr 0.7.2 on CRAN soon after.

Thanks !

larmarange commented 4 years ago

You should add rmarkdown in Suggests to avoid an issue with some checks, cf. https://github.com/juba/questionr/runs/1035915663?check_suite_focus=true

The new version of labelled has been validated by CRAN and should be available in few hours.

Anyway, you do not need to wait for for submitting questionr as the new code will work with any version of labelled.

Regards

juba commented 4 years ago

Ah, thanks, great suggestion.

Do you think we should add a dependency to labelled >= 2.6.0 ? Currently it is set at 2.4.0.

larmarange commented 4 years ago

questionr will work with both. However, the results of look_for() will not be consistent according to the version of labelled installed. Therefore, it could be relevant to add a >= 2.6.0 dependcy.

juba commented 4 years ago

Ok, I'll do that.

juba commented 4 years ago

0.7.2 is on its way to CRAN.