Skip to content

MAINT - Using pixi for nightlies and fixing failing nightly tests#1664

Merged
GaelVaroquaux merged 30 commits into
skrub-data:mainfrom
auguste-probabl:push-zlmrlrpontmz
Oct 29, 2025
Merged

MAINT - Using pixi for nightlies and fixing failing nightly tests#1664
GaelVaroquaux merged 30 commits into
skrub-data:mainfrom
auguste-probabl:push-zlmrlrpontmz

Conversation

@auguste-probabl

@auguste-probabl auguste-probabl commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

The strategy is to define a new pixi environment where the PyPI indexes provide the nightly builds. This is made difficult by pixi giving priority to conda channels rather than PyPI indexes when both are available. The solution I've found is to explicitly disable and redefine the dependencies, which is repetitive and thus error-prone.

Maybe pixi's conda-pypi-map could help; alternatively, maybe it's possible to find nightly dependencies in conda channels (after all, the scientific python ones do come from Anaconda)?

Closes #1173 and likely also closes #1586

@auguste-probabl auguste-probabl changed the title MNT: Switch to pixi for tests with nightly dependencies Switch to pixi for tests with nightly dependencies Oct 7, 2025
@auguste-probabl

Copy link
Copy Markdown
Contributor Author

I can't tell why nightly tests fail; my first guess would be that some dependency is missing from the nightly-dependencies environment, but I don't have evidence supporting this.

@rcap107

rcap107 commented Oct 9, 2025

Copy link
Copy Markdown
Member

Hi @auguste-probabl, I think this PR is already useful even if it is not perfect. I'm already glad I can run nightly tests with it!

I can reproduce the tests failing on my machine, I'll look into them

@rcap107

rcap107 commented Oct 9, 2025

Copy link
Copy Markdown
Member

Looking at the failed tests, it seems like the vast majority are doctests or unit tests that are failing because a RejectColumn error is raised. From what I can tell, the reason is the migration to str in pandas 3.0. At the moment, the dataframe API is checking for "object", so we'll have to update that for the new version (not looking forwards to that...).

https://pandas.pydata.org/pandas-docs/stable/user_guide/migration-3-strings.html#string-migration-guide

There are some additional tests that are failing and that might just be issues with other dependencies.

So what's happening is that nightly tests are failing because nightly dependencies will break things, so the tests are working as intended 🙈

I think we should merge this and start hammering whatever tests are being broken afterwards

WDYT @jeromedockes @GaelVaroquaux @glemaitre

rcap107
rcap107 previously approved these changes Oct 13, 2025
@rcap107 rcap107 dismissed their stale review October 13, 2025 09:18

Approval was done to test something, removing it for now

@rcap107

rcap107 commented Oct 16, 2025

Copy link
Copy Markdown
Member

I'll be using this PR to fix the bugs in the nightlies and merge this PR once the bugs have been addressed

@rcap107

rcap107 commented Oct 16, 2025

Copy link
Copy Markdown
Member

Unsuprisingly, fixing some tests broke some others.

Noting down things that are still failing:

  • A bunch of doctests where we print pandas series or dataframes, where the dtype is object on old versions and str in pandas 3.0
  • test_estimator fails in all cases (I think because of some change in scikit-learn 1.8)
  • test_parallel_coord_plot is failing for some reason I haven't figured out yet
  • test_common is failing because of the pandas changes
  • selectors fail because of the pandas changes
  • Same for test_clean_categories, test_empty_output, test_table_vectorizer, test_to_str

I'd like to have a way of replacing the values of certain docstrings depending on the version of pandas, but I couldn't find a decent way of doing it, which means I might have to use doctest +SKIP instead.

@GaelVaroquaux

GaelVaroquaux commented Oct 16, 2025 via email

Copy link
Copy Markdown
Member

@jeromedockes

Copy link
Copy Markdown
Member

maybe convert_dtypes() can help to have more consistent outputs by using the same ("nullable") dtypes across versions?

@jeromedockes

Copy link
Copy Markdown
Member

we could also consider running the doctests only in some environments (e.g. those with pandas < 3 for now), while making sure that all the behavior shown in doctests is also covered by unit tests (which run in all the environments, but can afford more boilerplate to handle discrepancies between versions)

see the --doctest-modules pytest option currently set in pyproject.toml

@rcap107 rcap107 added this to the 0.7.0 milestone Oct 20, 2025
@rcap107 rcap107 changed the title Switch to pixi for tests with nightly dependencies MAINT - Using pixi for nightlies and fixing failing nightly tests Oct 20, 2025
@rcap107

rcap107 commented Oct 23, 2025

Copy link
Copy Markdown
Member

I had to update the version of python because with py311 I was not able to pull the latest package versions (also it makes more sense to have a later version of python to test nightlies).

@rcap107

rcap107 commented Oct 23, 2025

Copy link
Copy Markdown
Member

This PR is blocked by #1618: the failing tests are in test_table_vectorizer, and the other PR is making a lot of changes in the same file so we should merge that one first.

@auguste-probabl

Copy link
Copy Markdown
Contributor Author

🙌

@GaelVaroquaux GaelVaroquaux left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only one comment: one place where I would like @rcap107 to add a comment explaining what's going on

Comment thread skrub/_dataframe/_common.py Outdated
Comment thread skrub/_dataframe/tests/test_common.py Outdated
assert dirpath.exists()


def test_extract_archive_exception_unlink_called():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused as to what these have to do with the subject of the PR, but that's not very important

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were an attempt at getting coverage to pass, but it did not work

I can remove the tests if you prefer

else:
out = transformer.fit(df).transform(df)
df_module.assert_frame_equal(out, s.select(df, ()))
selected = s.select(df, ())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful to add a comment explaining what the above line is doing (I cannot tell)

rcap107 and others added 3 commits October 29, 2025 08:48
Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
@GaelVaroquaux GaelVaroquaux merged commit 41b7245 into skrub-data:main Oct 29, 2025
29 checks passed
@GaelVaroquaux

Copy link
Copy Markdown
Member

Merged, thanks!

@auguste-probabl auguste-probabl deleted the push-zlmrlrpontmz branch October 29, 2025 09:06
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.

Test nightly dependencies fails all the time pixi environment using the nightly builds?

4 participants