extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
181 stars 27 forks source link

Remove warning about dev version when `use_dev_extendr = TRUE` #253

Closed etiennebacher closed 1 year ago

etiennebacher commented 1 year ago

Minor thing related to #250: I can use use_dev_extendr = TRUE in rust_source() but it still prints a message "Are you using a development version of extendr?". Is it possible to remove this message when I specify use_dev_extendr = TRUE?

packageVersion("rextendr")
#> [1] '0.2.0.9000'

rextendr::rust_source(
  code = "
  use either::Either::{self, Left, Right};
  #[extendr(use_try_from = true)]
  fn all_positive(input: Either<Integers, Doubles>) -> bool {
    match input {
      Left(ints) => ints.iter().all(|x| <Option<i32>>::from(x).unwrap_or(-1i32) >= 0i32),
      Right(dbls) => dbls.iter().all(|x| <Option<f64>>::from(x).unwrap_or(-1f64) >= 0f64),
    }
  }",
  features = "either", # Enable `extendr-api` feature `either`
  use_dev_extendr = TRUE, # Use `extendr-api` from github
  dependencies = list(either = "*"), # Reference `either` crate
)
#> Warning: Found unknown `extendr` feature: "either".
#> ℹ Are you using a development version of `extendr`?
#> ℹ build directory: 'C:/Users/etienne/AppData/Local/Temp/RtmpSAtVmS/filef0c3a4c4859'
#> ✔ Writing 'C:/Users/etienne/AppData/Local/Temp/RtmpSAtVmS/filef0c3a4c4859/target/extendr_wrappers.R'

Created on 2023-03-09 with reprex v2.0.2

etiennebacher commented 1 year ago

My bad, didn't see I could add quiet = TRUE

Ilia-Kosenkov commented 1 year ago

No worries! The issue here is that we have a set of 'known' features that we can handle (this list matches that of the released extendr-api), and whenever you provide an unknown feature (e.g. the one from the dev environment), we want to draw your attention to this fact. I will take a look if we can reduce the number of useless warnings while still keeping users informed.

etiennebacher commented 1 year ago

Maybe you could still indicate that one uses the dev version by replacing:

#> Warning: Found unknown `extendr` feature: "either".
#> ℹ Are you using a development version of `extendr`?

with

#> ℹ Using the development version of `extendr`: <commit>
#> ℹ Following features are not in release yet: "either".

Or maybe <commit> is useless because it uses the last one by default?

Ilia-Kosenkov commented 1 year ago

Yep, we just use the latest master by default. I get your point, I'll try rephrasing it :)

Ilia-Kosenkov commented 1 year ago

We improved notifications in #252, but those are related to function options. I will revisit features soon and improve warnings there as well.

Ilia-Kosenkov commented 1 year ago

Using this branch, now this should be perfectly silent

rextendr::rust_function(
 "fn all_positive(input: Either<Integers, Doubles>) -> bool {
    match input {
      Left(ints) => ints.iter().all(|x| <Option<i32>>::from(x).unwrap_or(-1i32) >= 0i32),
      Right(dbls) => dbls.iter().all(|x| <Option<f64>>::from(x).unwrap_or(-1f64) >= 0f64),
    }
  }",
  features = "either",
  extendr_fn_options = list(use_try_from = TRUE),
  use_dev_extendr = TRUE
)
#> ℹ build directory: 'C:/Users/.../AppData/Local/Temp/RtmpGoKV0K/file731c203f28de'
#> ✔ Writing 'C:/Users/.../AppData/Local/Temp/RtmpGoKV0K/file731c203f28de/target/extendr_wrappers.R'

-5:5 |> all_positive()
#> [1] FALSE
c(1, 2, 3, pi) |> all_positive()
#> [1] TRUE

Created on 2023-03-28 with reprex v2.0.2

This is being reviewed in #259.

etiennebacher commented 1 year ago

Nice, thank you! It works well with the code in the previous comment.

FYI, I now have an error with the original code. I've no idea if it's expected, just putting it here if it helps:

rextendr::rust_source(
  code = "
use either::Either::{self, Left, Right};
#[extendr(use_try_from = true)]
fn all_positive(input: Either<Integers, Doubles>) -> bool {
  match input {
    Left(ints) => ints.iter().all(|x| <Option<i32>>::from(x).unwrap_or(-1i32) >= 0i32),
    Right(dbls) => dbls.iter().all(|x| <Option<f64>>::from(x).unwrap_or(-1f64) >= 0f64),
  }
}",
  features = "either", # Enable `extendr-api` feature `either`
  use_dev_extendr = TRUE, # Use `extendr-api` from github
  dependencies = list(either = "*"), # Reference `either` crate
)
#> ℹ build directory:
#> 'C:/Users/etienne/AppData/Local/Temp/RtmpWQppaM/file24e0690d4ac0'
#> Error in `invoke_cargo()`:
#> ! Rust code could not be compiled successfully. Aborting.
#> ✖ error[E0659]: `either` is ambiguous
#>  --> src\lib.rs:3:5
#>   |
#> 3 | use either::Either::{self, Left, Right};
#>   |     ^^^^^^ ambiguous name
#>   |
#>   = note: ambiguous because of multiple potential import sources
#>   = note: `either` could refer to a crate passed with `--extern`
#>   = help: use `::either` to refer to this crate unambiguously
#> note: `either` could also refer to the module imported here
#>  --> src\lib.rs:1:5
#>   |
#> 1 | use extendr_api::prelude::*;
#>   |     ^^^^^^^^^^^^^^^^^^^^^^^
#>   = help: use `crate::either` to refer to this module unambiguously
#> ✖ error[E0603]: enum `Either` is private
#>   --> src\lib.rs:3:13
#>    |
#> 3  | use either::Either::{self, Left, Right};
#>    |             ^^^^^^ private enum
#>    |
#> note: the enum `Either` is defined here
#>   --> C:\Users\etienne\.cargo\git\checkouts\extendr-569c02e1a164cdf5\6e15390\extendr-api\src\optional\either.rs:32:5
#>    |
#> 32 | use crate::prelude::*;
#>    |     ^^^^^^^^^^^^^^^^^
#> ✖ error: aborting due to 2 previous errors
#> Backtrace:
#>     ▆
#>  1. └─rextendr::rust_source(...)
#>  2.   └─rextendr:::invoke_cargo(...)
#>  3.     └─rextendr:::check_cargo_output(...)
#>  4.       └─rextendr:::ui_throw(...)
#>  5.         └─rlang::abort(message, class = "rextendr_error", call = call)

Created on 2023-04-03 with reprex v2.0.2

Ilia-Kosenkov commented 1 year ago

We introduced a breaking change in extendr some time ago -- we agreed to re-export all third-party crates, which support is feature gated. This means that if you use extendr feature either, you no longer need to add either crate to dependencies, nor add a reference use either::Either::*, since it is re-rexported through extendr_api::prelude::*.