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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rust] PartialEq for DataFrame #1838

Merged
merged 2 commits into from
Nov 21, 2021
Merged

[Rust] PartialEq for DataFrame #1838

merged 2 commits into from
Nov 21, 2021

Conversation

ibENPC
Copy link
Contributor

@ibENPC ibENPC commented Nov 21, 2021

New implementation

Following #1833

impl PartialEq for DataFrame {
    fn eq(&self, other: &Self) -> bool {
        self.shape() == other.shape()
            && self
                .columns
                .par_iter()
                .zip_eq(other.columns.par_iter())
                .all(|(x, y)| x == y)
    }
}

Miri issue

Miri is giving me these warning and error:

test testing::test::test_df_partialeq ... warning: thread support is experimental and incomplete: weak memory effects are not emulated.

[...]

error: the main thread terminated without waiting for all remaining threads

I assume that the problem is because the eq function is parallel and is tested at the end of the Miri check, but I don't know why it doesn't wait for all the threads to finish 馃槄

@ibENPC
Copy link
Contributor Author

ibENPC commented Nov 21, 2021

There is some debate about this issue:

So I keep it simple and safe for now by not using the iterators of Rayon but the iterators of the std::iter.

impl PartialEq for DataFrame {
    fn eq(&self, other: &Self) -> bool {
        self.shape() == other.shape()
            && self
                .columns
                .iter()
                .zip(other.columns.iter())
                .all(|(s1, s2)| s1 == s2)
    }
}

@ritchie46
Copy link
Member

Also not needed to do in parallel. So great!

@ritchie46 ritchie46 merged commit be19af6 into pola-rs:master Nov 21, 2021
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.

None yet

2 participants