From 43a864f633bb0f16ca0875dd0964332a89c8bbd3 Mon Sep 17 00:00:00 2001 From: Nick Moore Date: Wed, 28 Aug 2024 09:15:54 +1000 Subject: [PATCH 1/4] add python3.12 tests to CI and tidy up --- .github/workflows/run-tests-push.yml | 156 ++++++--------------------- docs/contributing/index.md | 6 +- 2 files changed, 39 insertions(+), 123 deletions(-) diff --git a/.github/workflows/run-tests-push.yml b/.github/workflows/run-tests-push.yml index e71f440..75c4dcb 100644 --- a/.github/workflows/run-tests-push.yml +++ b/.github/workflows/run-tests-push.yml @@ -11,148 +11,60 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.9" + cache: 'pip' - run: sudo apt install xvfb - run: pip install --upgrade pip - run: pip install .[dev] - - run: xvfb-run pytest tests/ + - run: xvfb-run pytest -v -rP --doctest-modules countess/ tests/ - run-tests-ubuntu-22_04-python-3_10-with-coverage: + run-tests-ubuntu-22_04-python-3_10: runs-on: ubuntu-22.04 - name: Ubuntu 22.04, Python 3.10 (with coverage) + name: Ubuntu 22.04, Python 3.10 steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: python-version: "3.10" + cache: 'pip' - run: sudo apt install xvfb - run: pip install --upgrade pip - run: pip install .[dev] - - run: xvfb-run coverage run --source countess -m pytest tests/ -# - run: coverage html -# - uses: actions/upload-artifact@v3 -# with: -# name: test coverage report -# path: htmlcov/* - - run: echo '### Coverage Report' >> $GITHUB_STEP_SUMMARY - - run: coverage report --format=markdown --skip-empty --sort=-cover >> $GITHUB_STEP_SUMMARY - -# run-tests-ubuntu-22_04-python-3_11_0rc2: -# runs-on: ubuntu-22.04 -# name: Ubuntu 22.04, Python 3.11.0rc2 -# steps: -# - uses: actions/checkout@v4 -# - uses: actions/setup-python@v4 -# with: -# python-version: "3.11.0-rc.2" -# - run: sudo apt install xvfb -# - run: pip install --upgrade pip -# - run: pip install .[dev] -# - run: xvfb-run pytest tests/ -# -# run-tests-ubuntu-22_04-python-3_11_0: -# runs-on: ubuntu-22.04 -# name: Ubuntu 22.04, Python 3.11.1 -# steps: -# - uses: actions/checkout@v4 -# - uses: actions/setup-python@v4 -# with: -# python-version: "3.11.0" -# - run: sudo apt install xvfb -# - run: pip install --upgrade pip -# - run: pip install .[dev] -# - run: xvfb-run pytest tests/ -# -# run-tests-ubuntu-22_04-python-3_11_1: -# runs-on: ubuntu-22.04 -# name: Ubuntu 22.04, Python 3.11.1 -# steps: -# - uses: actions/checkout@v4 -# - uses: actions/setup-python@v4 -# with: -# python-version: "3.11.1" -# - run: sudo apt install xvfb -# - run: pip install --upgrade pip -# - run: pip install .[dev] -# - run: xvfb-run pytest tests/ - - run-tests-ubuntu-22_04-python-3_11_2: - runs-on: ubuntu-22.04 - name: Ubuntu 22.04, Python 3.11.2 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: "3.11.2" - - run: sudo apt install xvfb - - run: pip install --upgrade pip - - run: pip install .[dev] - - run: xvfb-run pytest tests/ - - run-tests-ubuntu-22_04-python-3_11: - runs-on: ubuntu-22.04 - name: Ubuntu 22.04, Python 3.11 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: "3.11" - - run: sudo apt install xvfb - - run: pip install --upgrade pip - - run: pip install .[dev] - - run: xvfb-run pytest tests/ - -# run-tests-ubuntu-22_04-python-3_12_0_alpha5: -# runs-on: ubuntu-22.04 -# name: Ubuntu 22.04, Python 3.12.0alpha5 -# steps: -# - uses: actions/checkout@v4 -# - uses: actions/setup-python@v4 -# with: -# python-version: "3.12.0-alpha.5" -# - run: sudo apt install xvfb -# - run: pip install --upgrade pip -# - run: pip install .[dev] -# - run: xvfb-run pytest tests/ + - run: xvfb-run pytest -v -rP --doctest-modules countess/ tests/ run-tests-ubuntu-22_04-python-3_11_from_apt: runs-on: ubuntu-22.04 - name: Ubuntu 22.04, Python from Apt + name: Ubuntu 22.04, Python 3.11 from Apt steps: - uses: actions/checkout@v4 - run: sudo apt install python3.11-full python3-pip xvfb - run: python3.11 -m pip install --upgrade pip - run: python3.11 -m pip install -e .[dev] - - run: xvfb-run python3.11 -mpytest tests/ + - run: xvfb-run python3.11 -m pytest -v -rP --doctest-modules countess/ tests/ - # run-tests-ubuntu-22_10-python-3_11_from_apt: - #runs-on: ubuntu-22.10 - #name: Ubuntu 22.10, Python from Apt - #steps: - #- uses: actions/checkout@v4 - #- run: sudo apt install python3.11-full python3-pip xvfb - #- run: python3.11 -m pip install --upgrade pip - #- run: python3.11 -m pip install -e .[dev] - #- run: xvfb-run python3.11 -mpytest tests/ - - #run-tests-ubuntu-23_04-python-3_11_from_apt: - #runs-on: ubuntu-23.04 - #name: Ubuntu 23.04, Python from Apt - #steps: - #- uses: actions/checkout@v4 - #- run: sudo apt install python3.11-full python3-pip xvfb - #- run: python3.11 -m pip install --upgrade pip - #- run: python3.11 -m pip install -e .[dev] - #- run: xvfb-run python3.11 -mpytest tests/ + run-tests-ubuntu-24_04-python-3_12_from_apt: + runs-on: ubuntu-24.04 + name: Ubuntu 24.04, Python 3.12 from Apt + steps: + - uses: actions/checkout@v4 + - run: sudo apt install python3.12-full python3-pip xvfb + - run: python3.12 -m venv /tmp/venv + - run: /tmp/venv/bin/python -m pip install --upgrade pip + - run: /tmp/venv/bin/python -m pip install -e .[dev] + - run: xvfb-run /tmp/venv/bin/python -m pytest -v -rP --doctest-modules countess/ tests/ -# run-tests-ubuntu-22_04-pypy3: -# runs-on: ubuntu-22.04 -# name: Ubuntu 22.04, PyPy 3 -# steps: -# - uses: actions/checkout@v4 -# - uses: actions/setup-python@v4 -# with: -# python-version: "pypy3.9" -# - run: sudo apt install pypy3 pypy3-tk pypy3-dev xvfb -# - run: pypy3 -mpip install -U pip wheel -# - run: pypy3 -mpip install .[dev] -# - run: xvfb-run pytest tests/ + run-tests-ubuntu-24_04-python-3_x: + runs-on: ubuntu-24.04 + name: Ubuntu 24.04, Python 3.x + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.x" + cache: 'pip' + - run: sudo apt install xvfb + - run: python3 -m venv /tmp/venv + - run: /tmp/venv/bin/python -m pip install --upgrade pip + - run: /tmp/venv/bin/python -m pip install -e .[dev] + - run: xvfb-run /tmp/venv/bin/coverage run --source countess -m pytest -v -rP --doctest-modules countess/ tests/ + - run: echo '### Coverage Report' >> $GITHUB_STEP_SUMMARY + - run: /tmp/venv/bin/coverage report --format=markdown --skip-empty --sort=-cover >> $GITHUB_STEP_SUMMARY diff --git a/docs/contributing/index.md b/docs/contributing/index.md index bda5e9c..9d71bd5 100644 --- a/docs/contributing/index.md +++ b/docs/contributing/index.md @@ -152,6 +152,9 @@ For issues with these pages, especially accessibility issues, please ## Deployment Github actions are set up to run code checks and tests on every push. +Tests are run across Python 3.9, 3.10, 3.11 and 3.12. Even though +3.9 is very old it is very widely deployed and there are a lot of +small but breaking changes. Deployment is not yet automated. There's a couple of small scripts to set a new version number in the code, documentation and git tags. @@ -162,7 +165,8 @@ name on the command line: script/set_version 1.2.3 -There's also a script to automate upload to PyPI using twine. +Releases are not yet automated. Releases are on PyPI (not github), +there's a script to automate upload to PyPI using twine: script/build_and_upload From 09e37de1d9775dd9bc7c9b300dacc2c39bf98252 Mon Sep 17 00:00:00 2001 From: Nick Moore Date: Thu, 29 Aug 2024 10:19:00 +1000 Subject: [PATCH 2/4] make logger setup for countess_cmd a bit more readable --- countess/core/cmd.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/countess/core/cmd.py b/countess/core/cmd.py index e934a20..b173060 100644 --- a/countess/core/cmd.py +++ b/countess/core/cmd.py @@ -17,15 +17,28 @@ def run(argv) -> None: def main() -> None: + # set up a default stderr StreamHandler for logs + logging_handler = logging.StreamHandler() + + # set up a QueueHandler/QueueListener to forward the logs between + # processes and send them to the logging_handler logging_queue: multiprocessing.Queue = multiprocessing.Queue() - logging.getLogger().addHandler(logging.handlers.QueueHandler(logging_queue)) - logging.getLogger().setLevel(logging.INFO) - logging_handler = logging.handlers.QueueListener(logging_queue, logging.StreamHandler()) - logging_handler.start() + logging_queue_handler = logging.handlers.QueueHandler(logging_queue) + logging_queue_listener = logging.handlers.QueueListener(logging_queue, logging_handler) + logging_queue_listener.start() + + # set up all loggers to be handled by the QueueHandler. + root_logger = logging.getLogger() + root_logger.addHandler(logging_queue_handler) + root_logger.setLevel(logging.INFO) run(sys.argv[1:]) - logging_handler.stop() + # shut down the logging subsystem, in case this function is being + # called as part of something else (eg: tests) + root_logger.handlers.clear() + logging_queue_listener.stop() + logging_queue.close() if __name__ == "__main__": From ffa73d5f98a2f9df487a18b48a1aff860adac59f Mon Sep 17 00:00:00 2001 From: Nick Moore Date: Thu, 29 Aug 2024 10:22:51 +1000 Subject: [PATCH 3/4] fixes for sentinelqueue and pipeline, tests --- countess/core/pipeline.py | 40 +++++----------- tests/plugins/test_filter.py | 26 +++++++++++ tests/test_pipeline.py | 91 ++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 29 deletions(-) create mode 100644 tests/plugins/test_filter.py create mode 100644 tests/test_pipeline.py diff --git a/countess/core/pipeline.py b/countess/core/pipeline.py index 94f6475..4b983f7 100644 --- a/countess/core/pipeline.py +++ b/countess/core/pipeline.py @@ -18,8 +18,12 @@ class SentinelQueue(Queue): The writer is expected to call `queue.finish()` when it is done and the reader can treat the queue like an iterable.""" - # XXX this is an attempt to handle multiple threads reading from the - # queue in parallel: they should all get StopIterations. + # catch attempts to 'put' more data onto the queue after it has finished. + finished = False + + # Handle multiple threads reading from the + # queue in parallel: once the sentinel has been received by any thread + # all further attempts to read get StopIterations. stopped = False class SENTINEL: @@ -27,6 +31,7 @@ class SENTINEL: def finish(self): self.put(self.SENTINEL) + self.finished = True def __iter__(self): return self @@ -47,13 +52,8 @@ def __next__(self): raise StopIteration return val - def get(self, block=True, timeout=None): - if self.stopped: - raise ValueError("SentinelQueue stopped") - return super().get(block, timeout) - def put(self, item, block=True, timeout=None): - if self.stopped: + if self.finished: raise ValueError("SentinelQueue stopped") super().put(item, block, timeout) @@ -83,7 +83,7 @@ def __init__(self, name, plugin=None, config=None, position=None, notes=None, so self.name = name self.plugin = plugin self.config = config or [] - self.position = position + self.position = position or (0.5, 0.5) self.sort_column = sort_column self.sort_descending = sort_descending self.notes = notes @@ -237,34 +237,16 @@ def del_parent(self, parent): parent.child_nodes.discard(self) self.mark_dirty() - def has_sibling(self): - return any(len(pn.child_nodes) > 1 for pn in self.parent_nodes) - def configure_plugin(self, key, value, base_dir="."): self.plugin.set_parameter(key, value, base_dir) self.mark_dirty() - def final_descendants(self): - if self.child_nodes: - return set(n2 for n1 in self.child_nodes for n2 in n1.final_descendants()) - else: - return set(self) - - def detatch(self): + def detach(self): for parent_node in self.parent_nodes: parent_node.child_nodes.discard(self) for child_node in self.child_nodes: child_node.parent_nodes.discard(self) - @classmethod - def get_ancestor_list(cls, nodes): - """Given a bunch of nodes, find the list of all the ancestors in a - sensible order""" - parents = set((p for n in nodes for p in n.parent_nodes)) - if not parents: - return list(nodes) - return cls.get_ancestor_list(parents) + list(nodes) - class PipelineGraph: def __init__(self): @@ -285,7 +267,7 @@ def add_node(self, node): self.nodes.append(node) def del_node(self, node): - node.detatch() + node.detach() self.nodes.remove(node) def traverse_nodes(self): diff --git a/tests/plugins/test_filter.py b/tests/plugins/test_filter.py new file mode 100644 index 0000000..31e7b0f --- /dev/null +++ b/tests/plugins/test_filter.py @@ -0,0 +1,26 @@ +import pandas as pd + +from countess.plugins.filter import FilterPlugin + +df1 = pd.DataFrame( + [ + {"foo": 1, "bar": 2, "baz": 3}, + {"foo": 4, "bar": 5, "baz": 6}, + {"foo": 7, "bar": 8, "baz": 9}, + ], +) + +df2 = df1.set_index("foo") + +code_1 = "qux = bar + baz\n\nquux = bar * baz\n" + +code_2 = "bar + baz != 11" + + +def test_filter_0(): + plugin = FilterPlugin() + # plugin.set_parameter("drop.1", True) + plugin.prepare(["x"]) + + dfs = list(plugin.process(df1, "x")) + assert len(dfs) == 0 diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py new file mode 100644 index 0000000..7be22c3 --- /dev/null +++ b/tests/test_pipeline.py @@ -0,0 +1,91 @@ +import pytest + +from countess.core.pipeline import PipelineNode, PipelineGraph + +@pytest.fixture(name='pg') +def fixture_pg(): + + pn0 = PipelineNode('node') + pn1 = PipelineNode('node') + pn2 = PipelineNode('node') + pn3 = PipelineNode('node') + pn4 = PipelineNode('node') + + pg = PipelineGraph() + pg.add_node(pn0) + pg.add_node(pn1) + pg.add_node(pn2) + pg.add_node(pn3) + pg.add_node(pn4) + + pn4.add_parent(pn2) + pn4.add_parent(pn3) + pn3.add_parent(pn1) + pn2.add_parent(pn1) + pn1.add_parent(pn0) + pn1.add_parent(pn0) + + return pg + +def test_ancestor_descendant(pg): + + pns = list(pg.traverse_nodes()) + for pn in pns[1:]: + assert pns[0].is_ancestor_of(pn) + assert not pn.is_ancestor_of(pns[0]) + + for pn in pns[:-1]: + assert pns[-1].is_descendant_of(pn) + assert not pn.is_descendant_of(pns[-1]) + + +def test_pipeline_graph_tidy(pg): + + pg.tidy() + + pns = list(pg.traverse_nodes()) + + # check that all nodes have different positions + assert len(set(pn.position for pn in pns)) == len(pns) + + # check that first coordinate is monotonic increasing + xs = [pn.position[0] for pn in pns] + assert sorted(xs) == xs + +def test_pipeline_del_node(pg): + + pns = list(pg.traverse_nodes()) + pg.del_node(pns[2]) + + assert not pns[2].is_descendant_of(pns[0]) + assert not pns[2].is_ancestor_of(pns[-1]) + +def test_pipeline_del_parent(pg): + + pns = list(pg.traverse_nodes()) + pns[2].del_parent(pns[1]) + + assert not pns[1].is_ancestor_of(pns[2]) + assert pns[2].is_ancestor_of(pns[-1]) + +def test_pipeline_graph_reset_node_name(pg): + + pns = list(pg.traverse_nodes()) + pg.reset_node_name(pns[1]) + assert pns[1].name == 'node 2' + + pg.reset_node_name(pns[3]) + assert pns[3].name == 'node 4' + +def test_pipeline_graph_reset_node_names(pg): + + pg.reset_node_names() + names = [pn.name for pn in pg.traverse_nodes()] + assert sorted(set(names)) == names + +def test_pg_reset(pg): + + pg.reset() + + assert all(pn.result is None for pn in pg.traverse_nodes()) + assert all(pn.is_dirty for pn in pg.traverse_nodes()) From 52b34235515fa060425b0792b6d1a951aa623df2 Mon Sep 17 00:00:00 2001 From: Nick Moore Date: Thu, 29 Aug 2024 10:24:53 +1000 Subject: [PATCH 4/4] code tidy --- tests/test_pipeline.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 7be22c3..889d681 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -1,15 +1,15 @@ import pytest -from countess.core.pipeline import PipelineNode, PipelineGraph +from countess.core.pipeline import PipelineGraph, PipelineNode -@pytest.fixture(name='pg') -def fixture_pg(): - pn0 = PipelineNode('node') - pn1 = PipelineNode('node') - pn2 = PipelineNode('node') - pn3 = PipelineNode('node') - pn4 = PipelineNode('node') +@pytest.fixture(name="pg") +def fixture_pg(): + pn0 = PipelineNode("node") + pn1 = PipelineNode("node") + pn2 = PipelineNode("node") + pn3 = PipelineNode("node") + pn4 = PipelineNode("node") pg = PipelineGraph() pg.add_node(pn0) @@ -27,8 +27,8 @@ def fixture_pg(): return pg -def test_ancestor_descendant(pg): +def test_ancestor_descendant(pg): pns = list(pg.traverse_nodes()) for pn in pns[1:]: assert pns[0].is_ancestor_of(pn) @@ -40,7 +40,6 @@ def test_ancestor_descendant(pg): def test_pipeline_graph_tidy(pg): - pg.tidy() pns = list(pg.traverse_nodes()) @@ -52,39 +51,39 @@ def test_pipeline_graph_tidy(pg): xs = [pn.position[0] for pn in pns] assert sorted(xs) == xs -def test_pipeline_del_node(pg): +def test_pipeline_del_node(pg): pns = list(pg.traverse_nodes()) pg.del_node(pns[2]) assert not pns[2].is_descendant_of(pns[0]) assert not pns[2].is_ancestor_of(pns[-1]) -def test_pipeline_del_parent(pg): +def test_pipeline_del_parent(pg): pns = list(pg.traverse_nodes()) pns[2].del_parent(pns[1]) assert not pns[1].is_ancestor_of(pns[2]) assert pns[2].is_ancestor_of(pns[-1]) -def test_pipeline_graph_reset_node_name(pg): +def test_pipeline_graph_reset_node_name(pg): pns = list(pg.traverse_nodes()) pg.reset_node_name(pns[1]) - assert pns[1].name == 'node 2' + assert pns[1].name == "node 2" pg.reset_node_name(pns[3]) - assert pns[3].name == 'node 4' + assert pns[3].name == "node 4" -def test_pipeline_graph_reset_node_names(pg): +def test_pipeline_graph_reset_node_names(pg): pg.reset_node_names() names = [pn.name for pn in pg.traverse_nodes()] assert sorted(set(names)) == names -def test_pg_reset(pg): +def test_pg_reset(pg): pg.reset() assert all(pn.result is None for pn in pg.traverse_nodes())