Skip to content

Commit

Permalink
improving / fixing linting / pre-commit hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
fzumstein committed Aug 14, 2023
1 parent 478e927 commit 9308228
Show file tree
Hide file tree
Showing 23 changed files with 90 additions and 94 deletions.
10 changes: 0 additions & 10 deletions .flake8

This file was deleted.

10 changes: 2 additions & 8 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,8 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: "pip"
- uses: psf/black@stable
with:
options: "--check --diff --color"
- uses: isort/isort-action@master
- run: pip install flake8
- run: flake8 .
python-version: "3.11"
- uses: pre-commit/action@v3.0.0

check-js-dist:
runs-on: ubuntu-latest
Expand Down
16 changes: 9 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ repos:
hooks:
- id: end-of-file-fixer

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.283
hooks:
- id: ruff
args:
- --config=./pyproject.toml

- repo: https://github.com/psf/black
rev: 23.3.0
hooks:
- id: black
args: [--config=./pyproject.toml]

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.282
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
args:
- --config=./pyproject.toml
3 changes: 1 addition & 2 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ To set up a development environment for the xlwings.js library, you need to do t
This repo uses the following packages for code formatting/linting, see `pyproject.toml`:

* black
* isort
* flake8
* ruff

You can use the pre-commit hook under `.pre-commit-config.yaml`, see instructions at top of the file.

Expand Down
4 changes: 2 additions & 2 deletions examples/mpl/mpl.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import xlwings as xw

try:
import seaborn
import seaborn # noqa: F401
except ImportError:
pass

Expand All @@ -36,4 +36,4 @@ def main():

# Get the figure and show it in Excel
fig = get_figure(const)
pic = sht.pictures.add(fig, name="MyStreamplot", update=True)
sht.pictures.add(fig, name="MyStreamplot", update=True)
43 changes: 27 additions & 16 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ build-backend = "maturin"

[project]
name = "xlwings"
description="Make Excel fly: Interact with Excel from Python and vice versa."
description = "Make Excel fly: Interact with Excel from Python and vice versa."
authors = [
{name ="Felix Zumstein", email = "felix.zumstein@zoomeranalytics.com"},
{ name = "Felix Zumstein", email = "felix.zumstein@zoomeranalytics.com" },
]
readme = "README.rst"
requires-python = ">=3.8"
Expand All @@ -29,15 +29,10 @@ classifiers = [
"License :: OSI Approved :: BSD License",
]

dynamic = [
"version"
]
dynamic = ["version"]

[project.optional-dependencies]
reports = [
"Jinja2",
"pdfrw"
]
reports = ["Jinja2", "pdfrw"]
all = [
"black",
"isort",
Expand All @@ -61,15 +56,31 @@ Funding = "https://www.xlwings.org/pricing"
Source = "https://github.com/xlwings/xlwings"
Changelog = "https://docs.xlwings.org/en/stable/whatsnew.html"

[tool.maturin]
exclude = ["xlwings/js/tsconfig.json", "xlwings/js/excel.d.ts"]

[tool.black]
target-version = ["py38"]
force-exclude = "xlwings/mistune"

[tool.isort]
profile = "black"
py_version=38
skip = "xlwings/mistune"
combine_as_imports = true
[tool.ruff]
target-version = "py38"
line-length = 88
fix = true
ignore = [
# Whitespace before ':' (black compatibility)
"E203",
# Line too long (black doesn't handle long strings)
"E501",
# Module level import not at top of file
"E402",
]
select = [
"E", # pycodestyle errors
"F", # pyflakes
"I", # isort
]

[tool.maturin]
exclude = ["xlwings/js/tsconfig.json", "xlwings/js/excel.d.ts"]
[tool.ruff.isort]
combine-as-imports = true
known-first-party = ["xlwings"]
14 changes: 7 additions & 7 deletions tests/permission/test_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ def test_permission_cant_override_config_file(clear_user_config, app, addin):

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/fail-machinename"\n',
f'"PERMISSION_CHECK_METHOD","POST"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/fail-machinename"\n',
'"PERMISSION_CHECK_METHOD","POST"\n',
'"UDF Modules","permission;permission2"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"',
]
Expand All @@ -191,7 +191,7 @@ def test_permission_udf_cant_find_file(clear_user_config, app, addin, method):

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
f'"PERMISSION_CHECK_METHOD","{method}"\n',
'"UDF Modules","permission;doesnexist"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"\n',
Expand Down Expand Up @@ -241,7 +241,7 @@ def test_permission_runpython_cant_find_file(clear_user_config, app, addin, meth

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
f'"PERMISSION_CHECK_METHOD","{method}"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"\n',
'"SHOW_ERROR_POPUPS","False"',
Expand All @@ -263,7 +263,7 @@ def test_permission_runpython_cant_use_from_imports(

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/success"\n',
f'"PERMISSION_CHECK_METHOD","{method}"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"\n',
'"SHOW_ERROR_POPUPS","False"',
Expand Down Expand Up @@ -348,7 +348,7 @@ def test_permission_embedded_code_success(clear_user_config, app, addin, method)

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/success-embedded"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/success-embedded"\n',
f'"PERMISSION_CHECK_METHOD","{method}"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"',
]
Expand Down Expand Up @@ -395,7 +395,7 @@ def test_permission_runpython_embedded_code_server_success(

config = [
'"PERMISSION_CHECK_ENABLED","True"\n',
f'"PERMISSION_CHECK_URL","http://localhost:5000/success-embedded"\n',
'"PERMISSION_CHECK_URL","http://localhost:5000/success-embedded"\n',
f'"PERMISSION_CHECK_METHOD","{method}"\n',
f'"LICENSE_KEY","{os.environ["TEST_XLWINGS_LICENSE_KEY"]}"\n',
'"USE UDF SERVER","True"',
Expand Down
2 changes: 1 addition & 1 deletion tests/restapi/test_restapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def get_book_urls(self, endpoint):
f"/apps/{self.app1.pid}/books/{self.wb1.name}" + endpoint,
f"/apps/{self.app1.pid}/books/0" + endpoint,
f"/books/{self.wb1.name}" + endpoint,
f"/books/0" + endpoint,
"/books/0" + endpoint,
f"/book/{self.wb1.name}" + endpoint,
f"/book/{self.wb1.fullname}" + endpoint,
]
Expand Down
4 changes: 2 additions & 2 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def test_active(self):
def test_len(self):
n_original = len(xw.apps)
app = xw.App(spec=SPEC)
wb = app.books.add()
app.books.add()
self.assertEqual(n_original + 1, len(xw.apps))
app.quit()

Expand Down Expand Up @@ -160,7 +160,7 @@ def test_len(self):
self.assertEqual(len(self.app1.books), n_books + 1)

def test_macro(self):
wb = self.app1.books.open(os.path.join(this_dir, "macro book.xlsm"))
self.app1.books.open(os.path.join(this_dir, "macro book.xlsm"))
test1 = self.app1.macro("Module1.Test1")
res1 = test1("Test1a", "Test1b")
self.assertEqual(res1, 1)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_book.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_indexing(self):

def test_len(self):
count = self.app1.books.count
wb = self.app1.books.add()
self.app1.books.add()
self.assertEqual(len(self.app1.books), count + 1)

def test_count(self):
Expand Down
16 changes: 8 additions & 8 deletions tests/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_udf_embedded_code(clear_user_config, addin, quickstart_book):


def test_can_use_xlwings_without_license_key(clear_user_config, tmp_path):
import xlwings
import xlwings # noqa: F401

os.chdir(tmp_path)
subprocess.run(split("xlwings quickstart testproject"))
Expand All @@ -154,16 +154,16 @@ def test_can_use_xlwings_without_license_key(clear_user_config, tmp_path):
def test_can_use_xlwings_with_wrong_license_key(clear_user_config, tmp_path):
os.makedirs(Path.home() / ".xlwings")
with open((Path.home() / ".xlwings" / "xlwings.conf"), "w") as config:
config.write(f'"LICENSE_KEY","xxx"')
import xlwings
config.write('"LICENSE_KEY","xxx"')
import xlwings # noqa: F401

os.chdir(tmp_path)
subprocess.run(split("xlwings quickstart testproject"))


def test_cant_use_xlwings_pro_without_license_key(clear_user_config):
with pytest.raises(xw.LicenseError):
import xlwings.pro
import xlwings.pro # noqa: F401


def test_addin_installation(app):
Expand Down Expand Up @@ -203,7 +203,7 @@ def test_standalone(clear_user_config, app, tmp_path):
@pytest.mark.skipif(xw.__version__ == "dev", reason="requires a built package")
def test_runpython_embedded_code_standalone(app, clear_user_config, tmp_path):
os.chdir(tmp_path)
subprocess.run(split(f"xlwings quickstart testproject --standalone"))
subprocess.run(split("xlwings quickstart testproject --standalone"))
quickstart_book = app.books.open(tmp_path / "testproject" / "testproject.xlsm")

os.makedirs(Path.home() / ".xlwings")
Expand All @@ -212,7 +212,7 @@ def test_runpython_embedded_code_standalone(app, clear_user_config, tmp_path):

os.chdir(tmp_path / "testproject")
subprocess.run(split("xlwings code embed"))
(tmp_path / "testproject" / f"testproject.py").unlink()
(tmp_path / "testproject" / "testproject.py").unlink()
sample_call = quickstart_book.macro("Module1.SampleCall")
sample_call()
assert quickstart_book.sheets[0]["A1"].value == "Hello xlwings!"
Expand All @@ -223,7 +223,7 @@ def test_runpython_embedded_code_standalone(app, clear_user_config, tmp_path):
@pytest.mark.skipif(xw.__version__ == "dev", reason="requires a built package")
def test_udf_embedded_code_standalone(clear_user_config, app, tmp_path):
os.chdir(tmp_path)
subprocess.run(split(f"xlwings quickstart testproject --standalone"))
subprocess.run(split("xlwings quickstart testproject --standalone"))
quickstart_book = app.books.open(tmp_path / "testproject" / "testproject.xlsm")

os.makedirs(Path.home() / ".xlwings")
Expand All @@ -232,7 +232,7 @@ def test_udf_embedded_code_standalone(clear_user_config, app, tmp_path):

os.chdir(tmp_path / "testproject")
subprocess.run(split("xlwings code embed"))
(tmp_path / "testproject" / f"testproject.py").unlink()
(tmp_path / "testproject" / "testproject.py").unlink()

quickstart_book.macro("ImportPythonUDFs")()
quickstart_book.sheets[0]["A1"].value = '=hello("test")'
Expand Down
2 changes: 1 addition & 1 deletion tests/test_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def test_sheet_scope(self):
self.wb2.sheets[0].range("B2:C3").value, [[1.0, 2.0], [3.0, 4.0]]
)
with self.assertRaises(Exception):
values = self.wb2.sheets[1].range("sheet_scope1").value
self.wb2.sheets[1].range("sheet_scope1").value

def test_workbook_scope(self):
self.wb1.sheets[0].range("A1").name = "test1"
Expand Down
28 changes: 14 additions & 14 deletions tests/test_shape.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def test_delete(self):

def test_type(self):
filename = os.path.join(this_dir, "sample_picture.png")
pic = self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.assertEqual(self.wb1.sheets[0].shapes[0].type, "picture")

def test_scale_width(self):
Expand Down Expand Up @@ -168,8 +168,8 @@ def test_delete(self):
def test_duplicate(self):
with self.assertRaises(xw.ShapeAlreadyExists):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic1")
pic2 = self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.wb1.sheets[0].pictures.add(filename, name="pic1")

def test_picture_update(self):
filename = os.path.join(this_dir, "sample_picture.png")
Expand All @@ -184,40 +184,40 @@ def test_picture_update_pathlib(self):

def test_picture_auto_update(self):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic1", update=True)
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic1", update=True)
self.wb1.sheets[0].pictures.add(filename, name="pic1", update=True)
self.wb1.sheets[0].pictures.add(filename, name="pic1", update=True)
self.assertEqual(len(self.wb1.sheets[0].pictures), 1)

def test_picture_auto_update_without_name(self):
with self.assertRaises(ValueError):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, update=True)
self.wb1.sheets[0].pictures.add(filename, update=True)

def test_picture_index(self):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.assertEqual(
self.wb1.sheets[0].pictures[0], self.wb1.sheets[0].pictures["pic1"]
)
self.assertEqual(self.wb1.sheets[0].pictures(1), self.wb1.sheets[0].pictures[0])

def test_len(self):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic1")
pic2 = self.wb1.sheets[0].pictures.add(filename, name="pic2")
self.wb1.sheets[0].pictures.add(filename, name="pic1")
self.wb1.sheets[0].pictures.add(filename, name="pic2")
self.assertEqual(len(self.wb1.sheets[0].pictures), 2)

def test_iter(self):
filename = os.path.join(this_dir, "sample_picture.png")
names = ["pic1", "pic2"]
pic1 = self.wb1.sheets[0].pictures.add(filename, name=names[0])
pic2 = self.wb1.sheets[0].pictures.add(filename, name=names[1])
self.wb1.sheets[0].pictures.add(filename, name=names[0])
self.wb1.sheets[0].pictures.add(filename, name=names[1])
for ix, pic in enumerate(self.wb1.sheets[0].pictures):
self.assertEqual(self.wb1.sheets[0].pictures[ix].name, names[ix])

def test_contains(self):
filename = os.path.join(this_dir, "sample_picture.png")
pic1 = self.wb1.sheets[0].pictures.add(filename, name="pic 1")
self.wb1.sheets[0].pictures.add(filename, name="pic 1")
self.assertTrue("pic 1" in self.wb1.sheets[0].pictures)


Expand Down Expand Up @@ -298,11 +298,11 @@ def test_add_properties(self):

class TestChart(TestBase):
def test_len(self):
chart = self.wb1.sheets[0].charts.add()
self.wb1.sheets[0].charts.add()
self.assertEqual(len(self.wb1.sheets[0].charts), 1)

def test_count(self):
chart = self.wb1.sheets[0].charts.add()
self.wb1.sheets[0].charts.add()
self.assertEqual(
len(self.wb1.sheets[0].charts), self.wb1.sheets[0].charts.count
)
Expand Down

0 comments on commit 9308228

Please sign in to comment.