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

CLEANUP: useNames=NA is now defunct; next is to drop code explaining that and just give an error #246

Closed
2 tasks done
HenrikBengtsson opened this issue Dec 11, 2023 · 12 comments
Milestone

Comments

@HenrikBengtsson
Copy link
Owner

HenrikBengtsson commented Dec 11, 2023

I've submitted matrixStats 1.2.0 (2023-12-11), which makes useNames = NA default.

There is a temporary, secret R option/env var to undefunct it, but that's just in case someone runs it to this and need an emergency fix. Next on the road map is to:

  • Remove all remaining code handling useNames = NA.
  • Anything but useNames = FALSE and useNames = TRUE should simply be an error.

This one should be a quick cleanup.

@HenrikBengtsson HenrikBengtsson added this to the Next release milestone Dec 11, 2023
@7788zzoo
Copy link

Hello! Our research project used "SingleR" package which is associated with "matrixStats". We met the problem that "useNames = NA is defunct.".
Could you please give us more detailed information about the temporary, secret R option/env var to undefunct it? We don't know the detailed data processing information about SingleR. Maybe we have to emergently change env var to solve this problem.
Thank you!

Our code is:

predictions <- SingleR(test=assay_for_singleR,

  •                    ref=cgData, labels=cgData$label.main)
    

Error: useNames = NA is defunct. Instead, specify either useNames = TRUE or useNames = FALSE.

@HenrikBengtsson
Copy link
Owner Author

You can still R_MATRIXSTATS_USENAMES_NA=deprecated.

That said, this option will soon be removed from matrixStats. After that, your only option is to install an older version of the package.

We don't know the detailed data processing information about SingleR.

Call traceback() when you get that error. That will help you narrow in on where the error occurs. I recommend trying to fix this in whatever code cases this. It's worth it, and it's always risky to rely on legacy code.

@HenrikBengtsson
Copy link
Owner Author

Will wait with this updated until after the next Bioconductor release 3.19 on 2024-05-01.

@HenrikBengtsson
Copy link
Owner Author

Our research project used "SingleR" package which is associated with "matrixStats". We met the problem that "useNames = NA is defunct.".

@7788zzoo , this is most likely because you're running an outdated version of Bioconductor. Upgrading would fix it. See
SingleR-inc/SingleR#256 for details and instructions.

@HenrikBengtsson
Copy link
Owner Author

Bioconductor 3.19 was release on 2024-05-01. We can now go ahead and clean out the special code handling useNames = NA. We should basically replace:

  if (is.na(useNames)) {
    deprecatedUseNamesNA()
  } else ...

with

  if (is.na(useNames)) {
    stop("Argument 'useNames' must not be NA")
  } else ...

everywhere.

@adRn-s
Copy link

adRn-s commented Jun 18, 2024

Where's the package changelog? Couldn't find any deprecation notice. I am looking for hints as to how to fix this... it migh be that there's no easy way around it, the broken downstream dependencies (SCTransform, glmGamPoi, SingleR, etc.) would need to be adjusted on their source code to add this function argument...

Perhaps switching to TRUE if the value was NA, with a warning, as to prevent breaking functionality would've been better. I am just saying this as a constructive criticism. Of course, I am just throwing my opinion with a lot of ignorance on the matter (from what I understand, the argument is not switching any important functionality for the downstream dependant packages.)

I'm using Bioc 3.16 with a matching CRAN snapshot for a course. I will probably downgrade to 1.1.0 on top of the package installation setup. If there's any better advice, a cleaner fix, I'd appreciate it.

@const-ae
Copy link
Contributor

const-ae commented Jun 18, 2024

Hi adRn's,

as the maintainer of glmGamPoi and contributor to MatrixGenerics, I am sorry to hear about the difficulties that you encountered with switch of the useNames arguments. I think the easiest option is to update MatrixGenerics:

devtools::install_github("Bioconductor/MatrixGenerics@RELEASE_3_18")

After that, I am able to run glmGamPoi on Bioconductor 3.16 with matrixStats version 1.2.0:

> y <- rpois(n = 100, lambda = 0.1)
> glmGamPoi::glm_gp(y)
glmGamPoiFit object:
The data had 1 rows and 100 columns.
A model with 1 coefficient was fitted.
> BiocManager::version()
[1] ‘3.16’
> packageVersion("matrixStats")
[1] ‘1.2.0’
> packageVersion("MatrixGenerics")
[1] ‘1.14.0’

And regarding your question about the changelog; it is in the NEWS.md file (as for every R package) : https://github.com/HenrikBengtsson/matrixStats/blob/develop/NEWS.md#deprecated-and-defunct-1

@HenrikBengtsson
Copy link
Owner Author

Perhaps switching to TRUE if the value was NA, with a warning, as to prevent breaking functionality would've been better. I am just saying this as a constructive criticism.

Agree, and this is what we have done over several years and release cycles. We really have tried to reach an as big audience as possible, but we knew that some will always find out much later and in years from now.

Installing an older version of matrixStats is one workaround. I'll try to mention this in the docs/news.

FYI, news(package = "matrixStats") is the canonical way in R to access the news/changelog.

@HenrikBengtsson
Copy link
Owner Author

I'm using Bioc 3.16 with a matching CRAN snapshot for a course.

I'd expect you get an older version of matrixStats that is compatible with Bioc 3.16 if you use CRAN snapshots. So, I'm surprised if you get a newer, incompatible version.

@adRn-s
Copy link

adRn-s commented Jun 18, 2024

Thanks for all of your helpful answers. It's under control now 👍🏽 ...Indeed, the CRAN snapshot was set incorrectly. I've just confirmed it. Another incompatibility was around (can't remember well what was it), so I moved the date forward a little... and that's how my frustration grew, jumping from one issue to the other (the usual debugging procedure, maybe). Anyway, thanks again for the support.

@gabrielascui
Copy link

Hi adRn's,

as the maintainer of glmGamPoi and contributor to MatrixGenerics, I am sorry to hear about the difficulties that you encountered with switch of the useNames arguments. I think the easiest option is to update MatrixGenerics:

devtools::install_github("Bioconductor/MatrixGenerics@RELEASE_3_18")

After that, I am able to run glmGamPoi on Bioconductor 3.16 with matrixStats version 1.2.0:

> y <- rpois(n = 100, lambda = 0.1)
> glmGamPoi::glm_gp(y)
glmGamPoiFit object:
The data had 1 rows and 100 columns.
A model with 1 coefficient was fitted.
> BiocManager::version()
[1] ‘3.16’
> packageVersion("matrixStats")
[1] ‘1.2.0’
> packageVersion("MatrixGenerics")
[1] ‘1.14.0’

And regarding your question about the changelog; it is in the NEWS.md file (as for every R package) : https://github.com/HenrikBengtsson/matrixStats/blob/develop/NEWS.md#deprecated-and-defunct-1

This solution worked for me too. Thank you!

@HenrikBengtsson
Copy link
Owner Author

useNames = NA is finally fully defunct and removed. There is no longer a workaround via options or env vars.

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

No branches or pull requests

5 participants