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

Updating IsNA trait #288

Merged
merged 13 commits into from
Sep 26, 2021
Merged

Conversation

Ilia-Kosenkov
Copy link
Member

@Ilia-Kosenkov Ilia-Kosenkov commented Sep 21, 2021

Trait IsNA is modified and renamed toCanBeNA.
New static function (i.e. function with no self arg) is introduced::
fn CanBeNA::na() -> Self
which can be used to obtain NA value of a given type.

Modified code in key places to switch to <type>::na() instead of helper functions.

I did not remove implementations of CanBeNA for primitive types (like f64 or i32).
This may be deprecated in the future.

@andy-thomason
Copy link
Contributor

Nice. Good to have a type-specific NA generator.

You can probably lose the turbofish <&std>:: in many cases as the type inference may do
this for you.

@Ilia-Kosenkov
Copy link
Member Author

Ilia-Kosenkov commented Sep 22, 2021

@andy-thomason,
Actually, it does not seem to work. &str::na(), for instance, is treated as &(str::na()), and there is no CanBeNA implementation for str. I, however, removed unnecessary <> for non-reference types like f64, but only in NA-related code.

Perhaps I am missing some Rust syntax feature.

@Ilia-Kosenkov
Copy link
Member Author

Ilia-Kosenkov commented Sep 22, 2021

CI fails due to 404 while getting R-devel for mac. I hate that whenever there is a version bump (or some other update) on CRAN side, it is always a wild 404 on one of the platforms.
Will re-run the CI later.

@andy-thomason andy-thomason merged commit 8c23f88 into extendr:master Sep 26, 2021
@andy-thomason andy-thomason linked an issue Oct 1, 2021 that may be closed by this pull request
@Ilia-Kosenkov Ilia-Kosenkov deleted the na-extension branch October 8, 2023 10:57
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 this pull request may close these issues.

Extend support for NA
3 participants