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

Add Python 3.11 to GHA #1090

Merged
merged 20 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/publish-python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.10'
python-version: '3.11'
- name: Install dependencies
run: |
python -m pip install --upgrade pip
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.9, "3.10"]
python-version: [3.9, "3.10", "3.11"]

steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ repos:
pyarrow>=1.0.1,
chardet>=3.0.4,
fastavro>=1.0.0.post1,
python-snappy>=0.5.4,
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove here? it is kept in requirements.txt file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

synced with requirements

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome

cramjam>=2.7.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be reverted 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

should match requirements.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

python-snappy>=0.7.1 supports Python 3.11 by switching over to cramjam and removing libsnappy-dev dependency (I contributed intake/python-snappy#130)

I will work dependencies in a minute

Copy link
Contributor

Choose a reason for hiding this comment

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

we reverted these cramjam changes due to failing tests locally though in #1122.

  • do we have to include cramjam + 3.11 in same pr?
  • I'm game for the 3.11 add
  • is the test suite running locally for you using DATAPROFILER_SEED=0 python3 -m unittest discover -p "test*.py" and tox -e py311?

charset-normalizer>=1.3.6,
psutil>=4.0.0,
scipy>=1.4.1,
Expand Down
16 changes: 0 additions & 16 deletions dataprofiler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,6 @@
from .validators.base_validators import Validator
from .version import __version__

try:
import snappy
except ImportError:
import warnings

warnings.warn(
"Snappy must be installed to use parquet/avro datasets."
"\n\n"
"For macOS use Homebrew:\n"
"\t`brew install snappy`"
"\n\n"
"For linux use apt-get:\n`"
"\tsudo apt-get -y install libsnappy-dev`\n",
ImportWarning,
)

taylorfturner marked this conversation as resolved.
Show resolved Hide resolved

def set_seed(seed=None):
# also check it's an integer
Expand Down
40 changes: 0 additions & 40 deletions dataprofiler/tests/test_data_profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,46 +56,6 @@ def test_data_profiling(self):
self.assertIsNotNone(profile.profile)
self.assertIsNotNone(profile.report())

def test_no_snappy(self):
import importlib
import sys
import types

orig_import = __import__
# necessary for any wrapper around the library to test if snappy caught
# as an issue

def reload_data_profiler():
"""Recursively reload modules."""
sys_modules = sys.modules.copy()
for module_name, module in sys_modules.items():
# Only reload top level of the dataprofiler
if "dataprofiler" in module_name and len(module_name.split(".")) < 3:
if isinstance(module, types.ModuleType):
importlib.reload(module)

def import_mock(name, *args, **kwargs):
if name == "snappy":
raise ImportError("test")
return orig_import(name, *args, **kwargs)

with mock.patch("builtins.__import__", side_effect=import_mock):
with self.assertWarns(ImportWarning) as w:
import dataprofiler

reload_data_profiler()

self.assertEqual(
str(w.warning),
"Snappy must be installed to use parquet/avro datasets."
"\n\n"
"For macOS use Homebrew:\n"
"\t`brew install snappy`"
"\n\n"
"For linux use apt-get:\n`"
"\tsudo apt-get -y install libsnappy-dev`\n",
)

taylorfturner marked this conversation as resolved.
Show resolved Hide resolved
def test_no_tensorflow(self):
import sys

Expand Down
2 changes: 1 addition & 1 deletion requirements-test.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
coverage>=5.0.1
dask>=2.29.0,<2024.2.0
dask[dask-expr,dataframe]>=2024.4.1
fsspec>=0.3.3
pytest>=6.0.1
pytest-cov>=2.8.1
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py39, py310, pypi-description, manifest, precom
envlist = py39, py310, py311, pypi-description, manifest, precom


[testenv]
Expand Down
Loading