Address intermittently failing tests

[stinodego]

There are some tests that fail occasionally:

• test_streaming_groupby_ooc
• test_polars_import

We should look into these to fix them, or skip them temporarily.

[mcrumiller]

Ok, I looked into the test_polars_import and I've found the issue:

        if_err = f"Possible import speed regression; took {total_import_time//1_000}ms"
        assert total_import_time < 200_000, f"{if_err}\n{df_import}"

The total import time was taking too long. The test runs the following:

python -X importtime -c "import polars"

And looking at the output, we have this:

FAILED tests/unit/test_polars_import.py::test_polars_import - AssertionError: Possible import speed regression; took 205ms
  ┌───────────────────────────────────────────────────────────────┬──────────┬─────────────────┐
  │ import                                                        ┆ own_time ┆ cumulative_time │
  │ ---                                                           ┆ ---      ┆ ---             │
  │ str                                                           ┆ i32      ┆ i32             │
  ╞═══════════════════════════════════════════════════════════════╪══════════╪═════════════════╡
  │  polars                                                       ┆ 1298     ┆ 123721          │
  |                                      ...                                                   |
  │        email                                                  ┆ 469      ┆ 469             │
  │  site                                                         ┆ 64801    ┆ 205091          │
  │    sitecustomize                                              ┆ 232      ┆ 232             │
  │    _sysconfigdata_m_linux_x86_64-linux-gnu                    ┆ 734      ┆ 734             │
  │    coverage                                                   ┆ 341      ┆ 122808          │
  │      coverage.control                                         ┆ 2058     ┆ 122007          │
                   ...

The site import is separate from polars. Looking at the docs for the site module, we have this:

This module is automatically imported during initialization. The automatic import can be suppressed using the interpreter’s -S option.

Importing this module will append site-specific paths to the module search path and add a few builtins, unless -S was used. In that case, this module can be safely imported with no automatic modifications to the module search path or additions to the builtins. To explicitly trigger the usual site-specific additions, call the site.main() function.

So, for our import test, I believe we should use the -S option, since we're only concerned with how long polars takes to load, not the site module, which is outside of our control.

[mcrumiller]

I'm going to make a separate issue for the first test since I have a PR to fix it and don't want to close this whole issue.

[ritchie46]

It is allowed to fail. It serves as a safeguard against import time regressions and there will be volatility.

[mcrumiller]

@ritchie46 the import time test is also testing the time it takes to import additional python modules; we should probably only look at the polars import time, no? It looks like we're getting false positives due to other modules taking longer than expected.

[stinodego]

I think the solution @mcrumiller is working on reduces volatility, which would be welcome.

Right now the false positive rate is too high - I just mindlessly rerun it without being worried about import times. So it's just noise right now.

[mcrumiller]

Yeah, this may take a little bit more research. I *think* the results from python -X importtime returns contiguous results related to individual packages, but I can't find that guarantee, and it's also hard to isolate polars alone. It also seems machine-dependent: when I run the command on my machine here, I get import times for io, zipimport. So one of the questions is: were these called by polars (I know io is used but zipimport is not), in which case we should probably include their load time.

Let me look a bit into this more and hopefully something will resolve.

[alexander-beedie]

I can address test_polars_import tonight, since that's one I added a little while ago (as an aside, one of the reasons it may fail locally a lot more often than in CI is because things can be a bit slower when running a debug build). Got back home last night, so am available once again... ;)

[mcrumiller]

@alexander-beedie I think the issue is that it's failing a lot more in CI than locally, at least that's what I've noticed. Might also be having a wailing fast machine helps...

[stinodego]

I think I know how to address the groupby one. It looks like the culprit are those temporary directories. Let's try to use pytest's tmp_path and see if that resolves some of our issues. I will pick this up later.

[stinodego]

Ah, looks like the test_streaming_groupby_ooc test actually doesn't use a temporary directory. So my idea does not affect this test.

That begs the question: where does this test write its data? Can we control where this is written somehow? The error states OSError: No such file or directory, so it must be related to this somehow.

@ritchie46 could you point me in the right direction here?

[ritchie46]

That begs the question: where does this test write its data?

Because it test out-of-core. The algorithm spills to disk when we run out of memory. Here we force it to take that code path.