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

Optionally warn for experimental behaviour #44

Closed
gowerc opened this issue Mar 17, 2020 · 9 comments · Fixed by #86
Closed

Optionally warn for experimental behaviour #44

gowerc opened this issue Mar 17, 2020 · 9 comments · Fixed by #86

Comments

@gowerc
Copy link

gowerc commented Mar 17, 2020

It would be good to be able to get code to issue warnings / errors if experimental functions have been used (the same way it does with depreciated functions). The aim being to highlight in potential production code where unstable functions have been used.

For example:

options(lifecycle_verbosity = "error_experimental")
@zeehio

This comment has been minimized.

@hadley

This comment has been minimized.

@hadley
Copy link
Member

hadley commented Jan 12, 2021

This is a bit more complex than I'd originally thought:

  • It's possible to have an non-experimental function that still uses an experimental function — while the implementation of the wrapping function might certainly change, the author can still promise that the interface won't. This is fairly common in the tidyverse where we want to play use experimental functions internally before sharing with the wider community.

  • An experimental function might use other experimental functions (and this will be quite common if there's a entire part of the API that's experimental).

In the first case you shouldn't get a warning and in the second you should only get one warning.

@hadley hadley changed the title Enhancement: Add ability to warn if code uses experimental functions Optionally wanr for experimental behaviour Jan 12, 2021
@hadley hadley changed the title Optionally wanr for experimental behaviour Optionally warn for experimental behaviour Jan 12, 2021
@hadley
Copy link
Member

hadley commented Jan 12, 2021

One way to implement it would be to provide experimental() and stable() wrappers that work with blocks of code, something like this:

fun1 <- function() {
  stable(fun2())
}

fun2 <- function() {
  experimental("fun2", {
    fun3()
  })
}

fun3 <- function() {
  experimental("fun3", 10)
}

But getting the combination of stable() and experimental() wrong will be easy, so you'd definitely want to test. That starts to feel like a lot of work for dubious benefit — if you're using experimental code in production, it's safer to use an isolated environment with pinned package versions.

@hadley
Copy link
Member

hadley commented Jan 12, 2021

The place where it's risky to use experimental functions is in packages that aren't under active development — if you use an experimental function you're basically making a promise to the developer of that function that you'll update your package if needed. That possibly suggests that surfacing use of experimental features is something that testthat could do?

@gowerc
Copy link
Author

gowerc commented Jan 13, 2021

I guess it depends who you want to build this feature for, i.e. end users vs package maintainers. The use case I had in mind when I made this ticket was for end users. The issue we have in my company is that a lot of our data scientists push code to production using experimental functions which, albeit work for that particular instance of the analysis, are likely to cause issues in the future if the interface changes. I was hoping that the proposed feature would be a way for us to push users away from using experimental functions in production bound code.

To your point though, in terms of our use case it doesn't matter if a non-experimental function has used an experimental backend as we'd assume the user interface will be consistent across future updates.

In terms of solutions, could you possibly tie it to the namespace ? i.e. only throw an error if it is an experimental function and it is a user facing (i.e. exported) function ?

@hadley
Copy link
Member

hadley commented Jan 13, 2021

I think we're going to introduce a new lifecycle::signal_stage() function that folks will be able to use to signal that a function is experimental. To begin with, this signal won't have any behaviour associated with it, but in the future we'll add some system so that you can log use of experimental functions. For the reasons described above, I don't think warnings are going to be effective without a lot of extra work.

Can you provide more details about what experimental functions they're using? I didn't think there were enough existing experimental functions for this to be an issue.

@gowerc
Copy link
Author

gowerc commented Jan 13, 2021

I must admit I don't have specific examples at the moment and that the issue is speculative. I just know my team keeps an active eye on the tidyverse and usually starts to experiment and use new features as soon as they are available. I have no doubt they will use any new experimental function in the future as soon as they are released so am proactively trying to guard against the above mentioned issue. Obviously this is more of a culture issue but technical solutions are always appreciated.

@hadley
Copy link
Member

hadley commented Jan 13, 2021

@gowerc not all new features are experimental; the experimental features are things that we are particularly uncertain about and are advertised as such (if we advertise them at all). I think you'd have to have a particularly danger-loving team for them to dive into experimental features right away 😄

hadley added a commit that referenced this issue Jan 15, 2021
Deprecates signal_experimental() and signal_superseded(), and removes lifecycle_cnd_data() (which isn't used anywhere on GitHub)

Fixes #44
hadley added a commit that referenced this issue Jan 23, 2021
Deprecates signal_experimental() and signal_superseded(), and removes lifecycle_cnd_data() (which isn't used anywhere on GitHub)

Fixes #44
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 a pull request may close this issue.

3 participants