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

CI: Ensure all minimum optional dependencies are tested #45103

Merged
merged 22 commits into from
Jan 5, 2022

Conversation

mroeschke
Copy link
Member

@mroeschke mroeschke commented Dec 29, 2021

  • tests added / passed

Progress towards #29685

  • actions-38-minimum_versions.yaml now contains all optional dependencies with their min versions
    • Other builds that have min version pinned are now removed
  • actions-38-db.yaml -> actions-38-downstream_compat.yaml
    • Build for compat with libraries like statsmodels, pytorch, yaml, etc
  • actions-38-db-min.yaml removed as redundant with actions-38-minimum_versions.yaml

Discoveries:
Min version of lxml and openpyxl with .xlsm or .xlsx doesn't seem to work. Need to bump to openpyxl to 3.0.9 https://openpyxl.readthedocs.io/en/stable/changes.html#id47

@mroeschke mroeschke marked this pull request as draft December 29, 2021 01:14
@jreback jreback added CI Continuous Integration Dependencies Required and optional dependencies labels Dec 31, 2021
@mroeschke mroeschke marked this pull request as ready for review January 2, 2022 19:24
@jreback jreback added this to the 1.4 milestone Jan 3, 2022
@jreback
Copy link
Contributor

jreback commented Jan 3, 2022

@mroeschke

Need to bump to openpyxl to 3.0.9

you updated the dev deps for this?

cc @lithomas1 can you review

@mroeschke
Copy link
Member Author

you updated the dev deps for this?

Don't think so. lxml and openpyxl don't have pins in environment.yml and requirements-dev.txt so the more recent versions should be installed.

@@ -289,7 +291,21 @@ def test_stata_options(fsspectest):


@td.skip_if_no("tabulate")
def test_markdown_options(fsspectest):
def test_markdown_options(request, fsspectest):
Copy link
Member

Choose a reason for hiding this comment

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

use @pytest.importorskip here with minversion?(pretty sure the issue here is with fsspec's min version only, but not sure).

Copy link
Member Author

Choose a reason for hiding this comment

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

With @pytest.importorskip I think I would have to specify the min version + 1 to guarantee skipping the min version. I think the +1 aspect isn't as simple as adding +1 to whatever comes out of __version__

- pip:
- py
Copy link
Member

Choose a reason for hiding this comment

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

Why add py? Also I'm pretty sure this is available on conda-forge. https://anaconda.org/conda-forge/py

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah cool I'll install the conda-forge version.

We have some tests that run on py.path objects

from py.path import local as LocalPath

Thought maybe in a follow up we should remove/discourage because it appears py.path is in maintenance only mode.

@jreback jreback merged commit a255173 into pandas-dev:master Jan 5, 2022
@jreback
Copy link
Contributor

jreback commented Jan 5, 2022

thanks @mroeschke @lithomas1

@mroeschke mroeschke deleted the ci/min_build branch January 5, 2022 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Dependencies Required and optional dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants