Change from assay_name to assay.type

[antagomir]

This should fix #189 as discussed.

The chance concerns only the arguments containing assay_name, changing these argument names from underscore type to dot type.

Changing all other argument names in this way, or reviewing other argument names where this would support interoperability with scuttle would be a heavier undertaking, perhaps best dealt with if specific needs will arise. Otherwise that will be mainly a stylistic issue.

[antagomir]

The latest commit adds the Deprecation messages.

Meanwhile I noticed that at least the following arguments are shared with or passed to scuttle, which has already moved to using dot names for arguments and deprecated these versions. I could add a similar change for these arguments in the same or another PR if you agree. It seems that the change should be done at the latest when scuttle removes these arguments from deprecation stage

• check_names (perFeatureDF arguments; within the ggfeatures function I already updated to call check.names for the scuttle call)
• use_altexps
• prefix_altexps
• use_dimred -> this is dimred in scater and use.dimred in scuttle

[alanocallaghan]

Can you revert to 6c30bee? I don't want to add deprecation messages within this cycle.

[alanocallaghan]

Also if you were adding deprecation messages the message should only pop up when somebody actually uses the old argument name. As-is people would get a deprecation message even if they were using the new argument name or not using either.

Something like

if (!missing(exprs_values)) {
  .Deprecated(msg="message")
}

[antagomir]

Ah I got it wrong when you gave the small comment that that the exprs_values is not deprecated. I thought it meant that the deprecation messages should be added.

I have now reverted to where it was before that comment.

I agree about the check before deprecation message, should have noticed that.

The question about the other arguments remains. I have listed above other arguments that scuttle has already deprecated (with message), and which are called in scater with their old names. It is possible to add those in this PR, or another new PR.

Is there something that still specifically has to be changed for this PR?

[alanocallaghan]

I haven't checked over the scuttle stuff yet, I think this PR is fine - if I want to change the other args I can do that myself anyways

[antagomir]

Ok great. Just let me know if anything still needs to be changed before merging. The checks seems to pass at least.

[antagomir]

The PR is ready from my side at least.

[alanocallaghan]

Can you see my review comments this time?

[antagomir]

I did not receive any notification on the review comments but now noticed them. I have added these in the latest push, hopefully all ok but if not then let's continue to finalize.

[alanocallaghan]

Thanks! Sorry for the delay in looking at it

[antagomir]

I would be looking fwd to having updates pushed to Bioconductor, then we could adopt these in downstream packages and tutorials.

[alanocallaghan]

Devel branch is pushed to bioc

[antagomir]

Awesome - thanks!