Skip to content

BLD: Add pyproject.toml to wheels #50330

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

Merged
merged 22 commits into from
Mar 20, 2023
Merged

Conversation

lithomas1
Copy link
Member

@lithomas1 lithomas1 commented Dec 17, 2022

@lithomas1 lithomas1 added the Build Library building on various platforms label Dec 19, 2022
@mroeschke
Copy link
Member

Thanks! Should we add a custom pytest.mark.smoketest mark in pyproject.toml so we can have a build that builds a wheel and runs pd.test with -m smoketest?

@lithomas1 lithomas1 marked this pull request as ready for review December 19, 2022 22:20
@lithomas1
Copy link
Member Author

Well, the wheel builder failures show that this definetely fixes it.

I've also done a slight modification to make --strict-data-files opt out instead of opt in. This also have the benefit of not showing a confusing no such option error, when you mistype in the path to a test you want to run. (you also can do just pytest now in the pandas directory, instead of doing pytest pandas)

@lithomas1
Copy link
Member Author

Thanks! Should we add a custom pytest.mark.smoketest mark in pyproject.toml so we can have a build that builds a wheel and runs pd.test with -m smoketest?

Sorry, can you clarify what a smoke test would mean in our test suite?

@mroeschke
Copy link
Member

Ah sorry. Not sure if it's too involved but a test in like pandas/tests/test_pd_test.py

@pytest.mark.smoketest
def test_pd_test_markers():
     assert True

Then something that runs pd.test(["-m smoketest"]) checking that only one test gets run?

@lithomas1
Copy link
Member Author

Ah sorry. Not sure if it's too involved but a test in like pandas/tests/test_pd_test.py

@pytest.mark.smoketest
def test_pd_test_markers():
     assert True

Then something that runs pd.test(["-m smoketest"]) checking that only one test gets run?

I think markers are still working, but just raising the unknown marker warning. I'll probably add a test to check that pyproject.toml can be found instead.

@lithomas1 lithomas1 requested a review from mroeschke December 21, 2022 00:41
@lithomas1 lithomas1 added this to the 2.0 milestone Dec 21, 2022
@mroeschke
Copy link
Member

Curious, why are the wheels only tested on windows?

Also might be worth to error on warnings from pytest when running the tests?

@lithomas1
Copy link
Member Author

lithomas1 commented Dec 30, 2022

Curious, why are the wheels only tested on windows?

Linux/macOS is tested by cibuildwheel directly here.
https://github.com/lithomas1/pandas/blob/a21dc267e0a2612a179e4f4878f5eacb7e4eab8a/pyproject.toml#L150

Windows is done in a separate step, since we test in an empty Docker container without MSVC installed to ensure that all DLLs are packaged in with the wheels.

Also might be worth to error on warnings from pytest when running the tests?

I'll look into enabling erroring on warnings.

@mroeschke
Copy link
Member

Looks like test_encoding_latin1_118 failed during wheel testing because a file wasn't found?

@mroeschke
Copy link
Member

Looks pretty good. Appears these tests need to filter out potential RuntimeWarnings

pandas/tests/series/methods/test_nlargest.py::TestSeriesNLargestNSmallest::test_nlargest_nullable
pandas/tests/frame/indexing/test_setitem.py::TestDataFrameSetItem::test_setitem_dtype

@mroeschke mroeschke modified the milestones: 2.0, 2.1 Mar 16, 2023
@lithomas1 lithomas1 requested a review from mroeschke March 19, 2023 00:28
@mroeschke mroeschke merged commit 8023225 into pandas-dev:main Mar 20, 2023
@mroeschke
Copy link
Member

Awesome! Thanks @lithomas1

@lithomas1 lithomas1 deleted the include-pyproject branch March 24, 2023 02:24
lithomas1 added a commit that referenced this pull request Mar 24, 2023
mroeschke pushed a commit that referenced this pull request Mar 24, 2023
* Revert "BLD: Add pyproject.toml to wheels (#50330)"

This reverts commit 8023225.

* don't revert test fixes

* fix whitespace

* more accidental whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: pd.test does not respect passing pandas specific markers
2 participants