From 688883390bff2c163e6b5300ebfaffe1575f3f0d Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 17:50:24 -0700 Subject: [PATCH 01/18] add basic telemetry --- docs/source/user_guide/index.md | 9 +++++++ examples/scripts/compare_lithium_ion.py | 1 + noxfile.py | 1 + src/pybamm/__init__.py | 5 ++-- src/pybamm/simulation.py | 3 +++ src/pybamm/telemetry.py | 33 +++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 src/pybamm/telemetry.py diff --git a/docs/source/user_guide/index.md b/docs/source/user_guide/index.md index 58ce04101a..166d241913 100644 --- a/docs/source/user_guide/index.md +++ b/docs/source/user_guide/index.md @@ -72,3 +72,12 @@ glob: ../examples/notebooks/creating_models/5-half-cell-model.ipynb ../examples/notebooks/creating_models/6-a-simple-SEI-model.ipynb ``` + +# Telemetry + +PyBaMM collects anonymous usage data to help improve the library. This telemetry is enabled by default but can be easily disabled. Here's what you need to know: + +- **What is collected**: Basic usage information like PyBaMM version, Python version, and which functions are run. +- **Why**: To understand how PyBaMM is used and prioritize development efforts. +- **Opt-out**: To disable telemetry, set the environment variable `PYBAMM_OPTOUT_TELEMETRY=true` or use `pybamm.telemetry.disable()` in your code. +- **Privacy**: No personal information (name, email, etc) or sensitive information (parameter values, simulation results) is ever collected. diff --git a/examples/scripts/compare_lithium_ion.py b/examples/scripts/compare_lithium_ion.py index 6108036b9b..4764778c34 100644 --- a/examples/scripts/compare_lithium_ion.py +++ b/examples/scripts/compare_lithium_ion.py @@ -4,6 +4,7 @@ import pybamm pybamm.set_logging_level("INFO") +pybamm.telemetry.disable() # load models models = [ diff --git a/noxfile.py b/noxfile.py index 6567ed167c..c71b906cb4 100644 --- a/noxfile.py +++ b/noxfile.py @@ -63,6 +63,7 @@ def set_iree_state(): "IREE_INDEX_URL": os.getenv( "IREE_INDEX_URL", "https://iree.dev/pip-release-links.html" ), + "PYBAMM_OPTOUT_TELEMETRY": "true", # don't capture telemetry in tests } VENV_DIR = Path("./venv").resolve() diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py index 36ad0b137a..53e2110153 100644 --- a/src/pybamm/__init__.py +++ b/src/pybamm/__init__.py @@ -192,8 +192,8 @@ # Batch Study from .batch_study import BatchStudy -# Callbacks -from . import callbacks +# Callbacks and telemetry +from . import callbacks, telemetry # Pybamm Data manager using pooch from .pybamm_data import DataLoader @@ -220,6 +220,7 @@ "simulation", "solvers", "spatial_methods", + "telemetry", "type_definitions", "util", "version", diff --git a/src/pybamm/simulation.py b/src/pybamm/simulation.py index da0ac08316..92f319e449 100644 --- a/src/pybamm/simulation.py +++ b/src/pybamm/simulation.py @@ -10,6 +10,7 @@ import warnings from functools import lru_cache from datetime import timedelta +import pybamm.telemetry from pybamm.util import import_optional_dependency from pybamm.expression_tree.operations.serialise import Serialise @@ -450,6 +451,8 @@ def solve( Additional key-word arguments passed to `solver.solve`. See :meth:`pybamm.BaseSolver.solve`. """ + pybamm.telemetry.capture("simulation-solved") + # Setup if solver is None: solver = self._solver diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py new file mode 100644 index 0000000000..d2110e4836 --- /dev/null +++ b/src/pybamm/telemetry.py @@ -0,0 +1,33 @@ +from posthog import Posthog +import os +import pybamm +import sys + +_posthog = Posthog( + # this is the public, write only API key, so it's ok to include it here + project_api_key="phc_bLZKBW03XjgiRhbWnPsnKPr0iw0z03fA6ZZYjxgW7ej", + host="https://us.i.posthog.com", +) + + +def disable(): + _posthog.disabled = True + + +_opt_out = os.getenv("PYBAMM_OPTOUT_TELEMETRY", "false").lower() +if _opt_out != "false": + disable() + + +def capture(event): + # setting $process_person_profile to False mean that we only track what events are + # being run and don't capture anything about the user + _posthog.capture( + "anonymous-user-id", + event, + properties={ + "$process_person_profile": False, + "python_version": sys.version, + "pybamm_version": pybamm.__version__, + }, + ) From a643c7b26d5098ba32fe7ec3fa06a04c5e43dc85 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 17:50:59 -0700 Subject: [PATCH 02/18] add posthog dependency --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 7fb1a5ce95..48317c7e2a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,7 @@ dependencies = [ "typing-extensions>=4.10.0", "pandas>=1.5.0", "pooch>=1.8.1", + "posthog", ] [project.urls] From c644876c1ac0a9ab68e23277a8c69dc0c418594f Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 18:30:46 -0700 Subject: [PATCH 03/18] ignore posthog url --- .lycheeignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.lycheeignore b/.lycheeignore index 55a4a4c623..929fe36475 100644 --- a/.lycheeignore +++ b/.lycheeignore @@ -15,3 +15,6 @@ file:///home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/fundamentals/pybam # Errors in docs/source/user_guide/index.md file:///home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/api_docs + +# Telemetry +https://us.i.posthog.com From 84a8a876ef526ef4a6c027cc393c9e9d882a9f36 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 18:35:00 -0700 Subject: [PATCH 04/18] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80cda39db7..faa0a83ea8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,12 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) ## Features + +- Added basic telemetry to record which functions are being run. See [Telemetry section in the User Guide](https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry) for more information. ([#4441](https://github.com/pybamm-team/PyBaMM/pull/4441)) - Added sensitivity calculation support for `pybamm.Simulation` and `pybamm.Experiment` ([#4415](https://github.com/pybamm-team/PyBaMM/pull/4415)) ## Optimizations + - Removed the `start_step_offset` setting and disabled minimum `dt` warnings for drive cycles with the (`IDAKLUSolver`). ([#4416](https://github.com/pybamm-team/PyBaMM/pull/4416)) ## Features @@ -11,6 +14,7 @@ - Added phase-dependent particle options to LAM #4369 ## Breaking changes + - The parameters "... electrode OCP entropic change [V.K-1]" and "... electrode volume change" are now expected to be functions of stoichiometry only instead of functions of both stoichiometry and maximum concentration ([#4427](https://github.com/pybamm-team/PyBaMM/pull/4427)) - Renamed `set_events` function to `add_events_from` to better reflect its purpose. ([#4421](https://github.com/pybamm-team/PyBaMM/pull/4421)) From cb2d075be9d05b98ea3365af91890eacbab2ea94 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 22:28:01 -0700 Subject: [PATCH 05/18] use uuid to identify individual users --- .gitignore | 1 - docs/source/user_guide/index.md | 2 +- pyproject.toml | 3 ++ scripts/post_install.py | 5 ++ src/pybamm/__init__.py | 5 +- src/pybamm/config.py | 85 +++++++++++++++++++++++++++++++++ src/pybamm/telemetry.py | 9 +++- 7 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 scripts/post_install.py create mode 100644 src/pybamm/config.py diff --git a/.gitignore b/.gitignore index 42c76b7c55..3bf8f02c0f 100644 --- a/.gitignore +++ b/.gitignore @@ -46,7 +46,6 @@ input/* # simulation outputs out/ -config.py matplotlibrc *.pickle *.sav diff --git a/docs/source/user_guide/index.md b/docs/source/user_guide/index.md index 166d241913..11d1e12ff7 100644 --- a/docs/source/user_guide/index.md +++ b/docs/source/user_guide/index.md @@ -80,4 +80,4 @@ PyBaMM collects anonymous usage data to help improve the library. This telemetry - **What is collected**: Basic usage information like PyBaMM version, Python version, and which functions are run. - **Why**: To understand how PyBaMM is used and prioritize development efforts. - **Opt-out**: To disable telemetry, set the environment variable `PYBAMM_OPTOUT_TELEMETRY=true` or use `pybamm.telemetry.disable()` in your code. -- **Privacy**: No personal information (name, email, etc) or sensitive information (parameter values, simulation results) is ever collected. +- **Privacy**: No personal information (name, email, etc) or sensitive information (parameter values, simulation results, etc) is ever collected. diff --git a/pyproject.toml b/pyproject.toml index 48317c7e2a..484c2bb334 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -281,3 +281,6 @@ module = [ "pybamm.models.full_battery_models.base_battery_model.*" ] disable_error_code = "attr-defined" + +[tool.poetry.scripts] +post-install = "scripts.post_install:run" diff --git a/scripts/post_install.py b/scripts/post_install.py new file mode 100644 index 0000000000..f9bd54e316 --- /dev/null +++ b/scripts/post_install.py @@ -0,0 +1,5 @@ +import pybamm + + +def run(): + pybamm.config.generate() diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py index 53e2110153..68b04c5e7e 100644 --- a/src/pybamm/__init__.py +++ b/src/pybamm/__init__.py @@ -192,8 +192,8 @@ # Batch Study from .batch_study import BatchStudy -# Callbacks and telemetry -from . import callbacks, telemetry +# Callbacks, telemetry, config +from . import callbacks, telemetry, config # Pybamm Data manager using pooch from .pybamm_data import DataLoader @@ -205,6 +205,7 @@ "batch_study", "callbacks", "citations", + "config", "discretisations", "doc_utils", "experiment", diff --git a/src/pybamm/config.py b/src/pybamm/config.py new file mode 100644 index 0000000000..b670f43257 --- /dev/null +++ b/src/pybamm/config.py @@ -0,0 +1,85 @@ +import uuid +import pathlib +import os + + +def _get_config_file(): + # Get the home directory + home_dir = pathlib.Path.home() + + # Create the file path for pybamm config + config_file = home_dir / ".config" / "pybamm" / "config.yml" + + return config_file + + +def is_running_tests(): + """ + Detect if the code is being run as part of a test suite. + + Returns: + bool: True if running tests, False otherwise. + """ + import sys + + # Check if pytest or unittest is running + if any( + test_module in sys.modules for test_module in ["pytest", "unittest", "nose"] + ): + return True + + # Check for GitHub Actions environment variable + if "GITHUB_ACTIONS" in os.environ: + return True + + # Check for other common CI environment variables + ci_env_vars = ["CI", "TRAVIS", "CIRCLECI", "JENKINS_URL", "GITLAB_CI"] + if any(var in os.environ for var in ci_env_vars): + return True + + # Check for common test runner names in command-line arguments + test_runners = ["pytest", "unittest", "nose", "trial", "nox", "tox"] + return any(runner in sys.argv[0].lower() for runner in test_runners) + + +def generate(): + if is_running_tests(): + return + + config_file = _get_config_file() + + # Create the directory if it doesn't exist + config_file.parent.mkdir(parents=True, exist_ok=True) + + # Check if the config file already exists + if read() is not None: + return + + # Generate a UUID + unique_id = uuid.uuid4() + + # Write the UUID to the config file in YAML format + with open(config_file, "w") as f: + f.write("pybamm:\n") + f.write(f" uuid: {unique_id}\n") + + +def read(): + config_file = _get_config_file() + + # Check if the config file exists + if not config_file.exists(): + return None + + # Read the UUID from the config file + with open(config_file) as f: + content = f.read().strip() + + # Extract the UUID using YAML parsing + try: + import yaml + + config = yaml.safe_load(content) + return config["pybamm"] + except (yaml.YAMLError, ValueError): + return None diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py index d2110e4836..830c0926b4 100644 --- a/src/pybamm/telemetry.py +++ b/src/pybamm/telemetry.py @@ -20,13 +20,18 @@ def disable(): def capture(event): + # don't capture events in automated testing + if pybamm.config.is_running_tests(): + return + + config = pybamm.config.read() + # setting $process_person_profile to False mean that we only track what events are # being run and don't capture anything about the user _posthog.capture( - "anonymous-user-id", + config["uuid"], event, properties={ - "$process_person_profile": False, "python_version": sys.version, "pybamm_version": pybamm.__version__, }, From a76aff7ef10883d95fb74ee9d54caf674d465e83 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 22:30:13 -0700 Subject: [PATCH 06/18] reordering --- src/pybamm/config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index b670f43257..3696c34cf2 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -46,15 +46,15 @@ def generate(): if is_running_tests(): return + # Check if the config file already exists + if read() is not None: + return + config_file = _get_config_file() # Create the directory if it doesn't exist config_file.parent.mkdir(parents=True, exist_ok=True) - # Check if the config file already exists - if read() is not None: - return - # Generate a UUID unique_id = uuid.uuid4() From 4bf6d80b5b488016374873691abacfbb35958efe Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 14 Sep 2024 22:37:01 -0700 Subject: [PATCH 07/18] add case where config is not found --- src/pybamm/telemetry.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py index 830c0926b4..7e8f588c8c 100644 --- a/src/pybamm/telemetry.py +++ b/src/pybamm/telemetry.py @@ -24,15 +24,18 @@ def capture(event): if pybamm.config.is_running_tests(): return + properties = { + "python_version": sys.version, + "pybamm_version": pybamm.__version__, + } + config = pybamm.config.read() + if config: + user_id = config["uuid"] + else: + user_id = "anonymous-user-id" + properties["$process_person_profile"] = False # setting $process_person_profile to False mean that we only track what events are # being run and don't capture anything about the user - _posthog.capture( - config["uuid"], - event, - properties={ - "python_version": sys.version, - "pybamm_version": pybamm.__version__, - }, - ) + _posthog.capture(user_id, event, properties=properties) From f7ec885d068e4d4f8acf1b410008ee143e3cd4c7 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sun, 15 Sep 2024 17:49:17 -0700 Subject: [PATCH 08/18] agriya comments and coverage --- conftest.py | 5 +++++ noxfile.py | 1 - src/pybamm/config.py | 31 ++++++++++++++----------------- src/pybamm/telemetry.py | 2 +- tests/unit/test_config.py | 30 ++++++++++++++++++++++++++++++ tests/unit/test_telemetry.py | 30 ++++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 tests/unit/test_config.py create mode 100644 tests/unit/test_telemetry.py diff --git a/conftest.py b/conftest.py index 7ac6cf3c74..77513d56db 100644 --- a/conftest.py +++ b/conftest.py @@ -51,3 +51,8 @@ def set_random_seed(): @pytest.fixture(autouse=True) def set_debug_value(): pybamm.settings.debug_mode = True + + +@pytest.fixture(autouse=True) +def disable_telemetry(): + pybamm.telemetry.disable() diff --git a/noxfile.py b/noxfile.py index c71b906cb4..6567ed167c 100644 --- a/noxfile.py +++ b/noxfile.py @@ -63,7 +63,6 @@ def set_iree_state(): "IREE_INDEX_URL": os.getenv( "IREE_INDEX_URL", "https://iree.dev/pip-release-links.html" ), - "PYBAMM_OPTOUT_TELEMETRY": "true", # don't capture telemetry in tests } VENV_DIR = Path("./venv").resolve() diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 3696c34cf2..be976cd2b7 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -1,19 +1,10 @@ import uuid -import pathlib import os +import platformdirs +from pathlib import Path -def _get_config_file(): - # Get the home directory - home_dir = pathlib.Path.home() - - # Create the file path for pybamm config - config_file = home_dir / ".config" / "pybamm" / "config.yml" - - return config_file - - -def is_running_tests(): +def is_running_tests(): # pragma: no cover """ Detect if the code is being run as part of a test suite. @@ -42,7 +33,7 @@ def is_running_tests(): return any(runner in sys.argv[0].lower() for runner in test_runners) -def generate(): +def generate(): # pragma: no cover if is_running_tests(): return @@ -50,8 +41,16 @@ def generate(): if read() is not None: return - config_file = _get_config_file() + config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml" + write_uuid_to_file(config_file) + +def read(): # pragma: no cover + config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml" + return read_uuid_from_file(config_file) + + +def write_uuid_to_file(config_file): # Create the directory if it doesn't exist config_file.parent.mkdir(parents=True, exist_ok=True) @@ -64,9 +63,7 @@ def generate(): f.write(f" uuid: {unique_id}\n") -def read(): - config_file = _get_config_file() - +def read_uuid_from_file(config_file): # Check if the config file exists if not config_file.exists(): return None diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py index 7e8f588c8c..69152f1a07 100644 --- a/src/pybamm/telemetry.py +++ b/src/pybamm/telemetry.py @@ -19,7 +19,7 @@ def disable(): disable() -def capture(event): +def capture(event): # pragma: no cover # don't capture events in automated testing if pybamm.config.is_running_tests(): return diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py new file mode 100644 index 0000000000..45c1260c09 --- /dev/null +++ b/tests/unit/test_config.py @@ -0,0 +1,30 @@ +import pytest + +import pybamm +import uuid + + +class TestConfig: + def test_write_read_uuid(self, tmp_path): + # Create a temporary file path + config_file = tmp_path / "config.yml" + + # Call the function to write UUID to file + pybamm.config.write_uuid_to_file(config_file) + + # Check that the file was created + assert config_file.exists() + + # Read the UUID using the read_uuid_from_file function + uuid_dict = pybamm.config.read_uuid_from_file(config_file) + + # Check that the UUID was read successfully + assert uuid_dict is not None + assert "uuid" in uuid_dict + + # Verify that the UUID is valid + + try: + uuid.UUID(uuid_dict["uuid"]) + except ValueError: + pytest.fail("Invalid UUID format") diff --git a/tests/unit/test_telemetry.py b/tests/unit/test_telemetry.py new file mode 100644 index 0000000000..45c1260c09 --- /dev/null +++ b/tests/unit/test_telemetry.py @@ -0,0 +1,30 @@ +import pytest + +import pybamm +import uuid + + +class TestConfig: + def test_write_read_uuid(self, tmp_path): + # Create a temporary file path + config_file = tmp_path / "config.yml" + + # Call the function to write UUID to file + pybamm.config.write_uuid_to_file(config_file) + + # Check that the file was created + assert config_file.exists() + + # Read the UUID using the read_uuid_from_file function + uuid_dict = pybamm.config.read_uuid_from_file(config_file) + + # Check that the UUID was read successfully + assert uuid_dict is not None + assert "uuid" in uuid_dict + + # Verify that the UUID is valid + + try: + uuid.UUID(uuid_dict["uuid"]) + except ValueError: + pytest.fail("Invalid UUID format") From abac35fafbb3af8bdea979b33f0b3ca546510dc6 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sun, 15 Sep 2024 18:25:13 -0700 Subject: [PATCH 09/18] more no cover --- src/pybamm/config.py | 4 ++-- src/pybamm/telemetry.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index be976cd2b7..e60ad1d5d2 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -65,7 +65,7 @@ def write_uuid_to_file(config_file): def read_uuid_from_file(config_file): # Check if the config file exists - if not config_file.exists(): + if not config_file.exists(): # pragma: no cover return None # Read the UUID from the config file @@ -78,5 +78,5 @@ def read_uuid_from_file(config_file): config = yaml.safe_load(content) return config["pybamm"] - except (yaml.YAMLError, ValueError): + except (yaml.YAMLError, ValueError): # pragma: no cover return None diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py index 69152f1a07..885249d22f 100644 --- a/src/pybamm/telemetry.py +++ b/src/pybamm/telemetry.py @@ -15,7 +15,7 @@ def disable(): _opt_out = os.getenv("PYBAMM_OPTOUT_TELEMETRY", "false").lower() -if _opt_out != "false": +if _opt_out != "false": # pragma: no cover disable() From a41d3ab66ef57690efb84e5a20178374f6841803 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 2 Oct 2024 20:50:18 -0700 Subject: [PATCH 10/18] update telemetry to opt-in --- docs/source/user_guide/index.md | 4 +-- examples/scripts/compare_lithium_ion.py | 1 - scripts/post_install.py | 5 --- src/pybamm/__init__.py | 3 ++ src/pybamm/config.py | 46 +++++++++++++++++++++---- src/pybamm/telemetry.py | 15 +++----- tests/unit/test_config.py | 38 +++++++++++++------- 7 files changed, 75 insertions(+), 37 deletions(-) delete mode 100644 scripts/post_install.py diff --git a/docs/source/user_guide/index.md b/docs/source/user_guide/index.md index 11d1e12ff7..f61d2fc253 100644 --- a/docs/source/user_guide/index.md +++ b/docs/source/user_guide/index.md @@ -75,9 +75,9 @@ glob: # Telemetry -PyBaMM collects anonymous usage data to help improve the library. This telemetry is enabled by default but can be easily disabled. Here's what you need to know: +PyBaMM optionally collects anonymous usage data to help improve the library. This telemetry is opt-in and can be easily disabled. Here's what you need to know: - **What is collected**: Basic usage information like PyBaMM version, Python version, and which functions are run. - **Why**: To understand how PyBaMM is used and prioritize development efforts. -- **Opt-out**: To disable telemetry, set the environment variable `PYBAMM_OPTOUT_TELEMETRY=true` or use `pybamm.telemetry.disable()` in your code. +- **Opt-out**: To disable telemetry, set the environment variable `PYBAMM_DISABLE_TELEMETRY=true` (or any value other than `false`) or use `pybamm.telemetry.disable()` in your code. - **Privacy**: No personal information (name, email, etc) or sensitive information (parameter values, simulation results, etc) is ever collected. diff --git a/examples/scripts/compare_lithium_ion.py b/examples/scripts/compare_lithium_ion.py index 4764778c34..6108036b9b 100644 --- a/examples/scripts/compare_lithium_ion.py +++ b/examples/scripts/compare_lithium_ion.py @@ -4,7 +4,6 @@ import pybamm pybamm.set_logging_level("INFO") -pybamm.telemetry.disable() # load models models = [ diff --git a/scripts/post_install.py b/scripts/post_install.py deleted file mode 100644 index f9bd54e316..0000000000 --- a/scripts/post_install.py +++ /dev/null @@ -1,5 +0,0 @@ -import pybamm - - -def run(): - pybamm.config.generate() diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py index 3a6f7eeddd..643248f5c7 100644 --- a/src/pybamm/__init__.py +++ b/src/pybamm/__init__.py @@ -227,3 +227,6 @@ "version", "pybamm_data", ] + + +pybamm.config.generate() diff --git a/src/pybamm/config.py b/src/pybamm/config.py index e60ad1d5d2..9e3029851b 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -2,6 +2,7 @@ import os import platformdirs from pathlib import Path +import pybamm def is_running_tests(): # pragma: no cover @@ -33,6 +34,34 @@ def is_running_tests(): # pragma: no cover return any(runner in sys.argv[0].lower() for runner in test_runners) +def ask_user_opt_in(): + """ + Ask the user if they want to opt in to telemetry. + + Returns + ------- + bool + True if the user opts in, False otherwise. + """ + print( + "PyBaMM can collect usage data and send it to the PyBaMM team to " + "help us improve the software.\n" + "We do not collect any sensitive information such as models, parameters, " + "or simulation results - only information on which parts of the code are " + "being used and how frequently.\n" + "This is entirely optional and does not impact the functionality of PyBaMM.\n" + "For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry" + ) + while True: + user_input = input("Do you want to enable telemetry? (Y/n): ").strip().lower() + if user_input in ["yes", "y"]: + return True + elif user_input in ["no", "n"]: + return False + else: + print("\nInvalid input. Please enter 'yes'/'y' or 'no'/'n'.") + + def generate(): # pragma: no cover if is_running_tests(): return @@ -41,8 +70,13 @@ def generate(): # pragma: no cover if read() is not None: return + # Ask the user if they want to opt in to telemetry + opt_in = ask_user_opt_in() config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml" - write_uuid_to_file(config_file) + write_uuid_to_file(config_file, opt_in) + + if opt_in: + pybamm.telemetry.capture("user-opted-in") def read(): # pragma: no cover @@ -50,17 +84,17 @@ def read(): # pragma: no cover return read_uuid_from_file(config_file) -def write_uuid_to_file(config_file): +def write_uuid_to_file(config_file, opt_in): # Create the directory if it doesn't exist config_file.parent.mkdir(parents=True, exist_ok=True) - # Generate a UUID - unique_id = uuid.uuid4() - # Write the UUID to the config file in YAML format with open(config_file, "w") as f: f.write("pybamm:\n") - f.write(f" uuid: {unique_id}\n") + f.write(f" enable_telemetry: {opt_in}\n") + if opt_in: + unique_id = uuid.uuid4() + f.write(f" uuid: {unique_id}\n") def read_uuid_from_file(config_file): diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py index 885249d22f..af91deca9d 100644 --- a/src/pybamm/telemetry.py +++ b/src/pybamm/telemetry.py @@ -14,14 +14,14 @@ def disable(): _posthog.disabled = True -_opt_out = os.getenv("PYBAMM_OPTOUT_TELEMETRY", "false").lower() +_opt_out = os.getenv("PYBAMM_DISABLE_TELEMETRY", "false").lower() if _opt_out != "false": # pragma: no cover disable() def capture(event): # pragma: no cover # don't capture events in automated testing - if pybamm.config.is_running_tests(): + if pybamm.config.is_running_tests() or _posthog.disabled: return properties = { @@ -31,11 +31,6 @@ def capture(event): # pragma: no cover config = pybamm.config.read() if config: - user_id = config["uuid"] - else: - user_id = "anonymous-user-id" - properties["$process_person_profile"] = False - - # setting $process_person_profile to False mean that we only track what events are - # being run and don't capture anything about the user - _posthog.capture(user_id, event, properties=properties) + if config["enable_telemetry"]: + user_id = config["uuid"] + _posthog.capture(user_id, event, properties=properties) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 45c1260c09..68610f48ee 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -5,26 +5,38 @@ class TestConfig: - def test_write_read_uuid(self, tmp_path): + @pytest.mark.parametrize("write_opt_in", [True, False]) + def test_write_read_uuid(self, tmp_path, write_opt_in): # Create a temporary file path config_file = tmp_path / "config.yml" # Call the function to write UUID to file - pybamm.config.write_uuid_to_file(config_file) + pybamm.config.write_uuid_to_file(config_file, write_opt_in) # Check that the file was created assert config_file.exists() # Read the UUID using the read_uuid_from_file function - uuid_dict = pybamm.config.read_uuid_from_file(config_file) - + config_dict = pybamm.config.read_uuid_from_file(config_file) # Check that the UUID was read successfully - assert uuid_dict is not None - assert "uuid" in uuid_dict - - # Verify that the UUID is valid - - try: - uuid.UUID(uuid_dict["uuid"]) - except ValueError: - pytest.fail("Invalid UUID format") + if write_opt_in: + assert config_dict["enable_telemetry"] is True + assert "uuid" in config_dict + + # Verify that the UUID is valid + try: + uuid.UUID(config_dict["uuid"]) + except ValueError: + pytest.fail("Invalid UUID format") + else: + assert config_dict["enable_telemetry"] is False + + def test_ask_user_opt_in(self, monkeypatch): + # Mock the input function to always return "y" + monkeypatch.setattr("builtins.input", lambda _: "y") + + # Call the function to ask the user if they want to opt in + opt_in = pybamm.config.ask_user_opt_in() + + # Check that the function returns True + assert opt_in is True From 3939e7f34130f398fa3ed805df981d6c27e87575 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 2 Oct 2024 23:12:02 -0700 Subject: [PATCH 11/18] remove duplicate test --- tests/unit/test_telemetry.py | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 tests/unit/test_telemetry.py diff --git a/tests/unit/test_telemetry.py b/tests/unit/test_telemetry.py deleted file mode 100644 index 45c1260c09..0000000000 --- a/tests/unit/test_telemetry.py +++ /dev/null @@ -1,30 +0,0 @@ -import pytest - -import pybamm -import uuid - - -class TestConfig: - def test_write_read_uuid(self, tmp_path): - # Create a temporary file path - config_file = tmp_path / "config.yml" - - # Call the function to write UUID to file - pybamm.config.write_uuid_to_file(config_file) - - # Check that the file was created - assert config_file.exists() - - # Read the UUID using the read_uuid_from_file function - uuid_dict = pybamm.config.read_uuid_from_file(config_file) - - # Check that the UUID was read successfully - assert uuid_dict is not None - assert "uuid" in uuid_dict - - # Verify that the UUID is valid - - try: - uuid.UUID(uuid_dict["uuid"]) - except ValueError: - pytest.fail("Invalid UUID format") From b8f588b6b37168fb2a2c7e09a0d77b02b25faa1b Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 2 Oct 2024 23:55:39 -0700 Subject: [PATCH 12/18] coverage --- src/pybamm/config.py | 13 ++++++++++--- tests/unit/test_config.py | 12 ++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 9e3029851b..8dd1b56e75 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -7,10 +7,10 @@ def is_running_tests(): # pragma: no cover """ - Detect if the code is being run as part of a test suite. + Detect if the code is being run as part of a test suite or building docs with Sphinx. Returns: - bool: True if running tests, False otherwise. + bool: True if running tests or building docs, False otherwise. """ import sys @@ -31,7 +31,14 @@ def is_running_tests(): # pragma: no cover # Check for common test runner names in command-line arguments test_runners = ["pytest", "unittest", "nose", "trial", "nox", "tox"] - return any(runner in sys.argv[0].lower() for runner in test_runners) + if any(runner in sys.argv[0].lower() for runner in test_runners): + return True + + # Check if building docs with Sphinx + if "sphinx" in sys.modules: + return True + + return False def ask_user_opt_in(): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 68610f48ee..212d0eef78 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -31,12 +31,12 @@ def test_write_read_uuid(self, tmp_path, write_opt_in): else: assert config_dict["enable_telemetry"] is False - def test_ask_user_opt_in(self, monkeypatch): - # Mock the input function to always return "y" - monkeypatch.setattr("builtins.input", lambda _: "y") + @pytest.mark.parametrize("user_opted_in, user_input", [(True, "y"), (False, "n")]) + def test_ask_user_opt_in(self, monkeypatch, user_opted_in, user_input): + # Mock the input function to return invalid input first, then valid input + inputs = iter(["invalid", user_input]) + monkeypatch.setattr("builtins.input", lambda _: next(inputs)) # Call the function to ask the user if they want to opt in opt_in = pybamm.config.ask_user_opt_in() - - # Check that the function returns True - assert opt_in is True + assert opt_in is user_opted_in From 2e222c6d445759791aef8fed57fe917d2b50ffc8 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 5 Oct 2024 11:24:42 -0400 Subject: [PATCH 13/18] implement timeout --- pyproject.toml | 1 + src/pybamm/config.py | 24 +++++++++++++++++++++--- tests/unit/test_config.py | 18 ++++++++++++++++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1a527f783d..46c8850bc7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ dependencies = [ "pandas>=1.5.0", "pooch>=1.8.1", "posthog", + "inputimeout", ] [project.urls] diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 8dd1b56e75..5095b42324 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -3,6 +3,7 @@ import platformdirs from pathlib import Path import pybamm +from inputimeout import inputimeout, TimeoutOccurred def is_running_tests(): # pragma: no cover @@ -41,10 +42,15 @@ def is_running_tests(): # pragma: no cover return False -def ask_user_opt_in(): +def ask_user_opt_in(timeout=10): """ Ask the user if they want to opt in to telemetry. + Parameters + ---------- + timeout: float, optional + The timeout for the user to respond to the prompt. Default is 20 seconds. + Returns ------- bool @@ -59,9 +65,21 @@ def ask_user_opt_in(): "This is entirely optional and does not impact the functionality of PyBaMM.\n" "For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry" ) + while True: - user_input = input("Do you want to enable telemetry? (Y/n): ").strip().lower() - if user_input in ["yes", "y"]: + try: + user_input = ( + inputimeout( + prompt="Do you want to enable telemetry? (Y/n): ", timeout=timeout + ) + .strip() + .lower() + ) + except TimeoutOccurred: + print("\nTimeout reached. Defaulting to not enabling telemetry.") + return False + + if user_input in ["yes", "y", ""]: return True elif user_input in ["no", "n"]: return False diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 212d0eef78..6e262fb011 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -1,4 +1,5 @@ import pytest +from inputimeout import TimeoutOccurred import pybamm import uuid @@ -33,10 +34,23 @@ def test_write_read_uuid(self, tmp_path, write_opt_in): @pytest.mark.parametrize("user_opted_in, user_input", [(True, "y"), (False, "n")]) def test_ask_user_opt_in(self, monkeypatch, user_opted_in, user_input): - # Mock the input function to return invalid input first, then valid input + # Mock the inputimeout function to return invalid input first, then valid input inputs = iter(["invalid", user_input]) - monkeypatch.setattr("builtins.input", lambda _: next(inputs)) + monkeypatch.setattr( + "pybamm.config.inputimeout", lambda prompt, timeout: next(inputs) + ) # Call the function to ask the user if they want to opt in opt_in = pybamm.config.ask_user_opt_in() assert opt_in is user_opted_in + + def test_ask_user_opt_in_timeout(self, monkeypatch): + # Mock the inputimeout function to raise a TimeoutOccurred exception + def mock_inputimeout(*args, **kwargs): + raise TimeoutOccurred + + monkeypatch.setattr("pybamm.config.inputimeout", mock_inputimeout) + + # Call the function to ask the user if they want to opt in + opt_in = pybamm.config.ask_user_opt_in() + assert opt_in is False From d48f8c63c57c629c855811160ef02fea416e80ac Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Sat, 12 Oct 2024 16:22:06 -0400 Subject: [PATCH 14/18] Update src/pybamm/config.py Co-authored-by: Eric G. Kratz --- src/pybamm/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 5095b42324..222b1d8da2 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -49,7 +49,7 @@ def ask_user_opt_in(timeout=10): Parameters ---------- timeout: float, optional - The timeout for the user to respond to the prompt. Default is 20 seconds. + The timeout for the user to respond to the prompt. Default is 10 seconds. Returns ------- From 815bf91ec3ea37085ebbc23aee2e61d9c3e936f6 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Fri, 18 Oct 2024 14:03:34 -0400 Subject: [PATCH 15/18] 10 second timeout --- src/pybamm/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 5095b42324..222b1d8da2 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -49,7 +49,7 @@ def ask_user_opt_in(timeout=10): Parameters ---------- timeout: float, optional - The timeout for the user to respond to the prompt. Default is 20 seconds. + The timeout for the user to respond to the prompt. Default is 10 seconds. Returns ------- From 8d1e192499d475f70eab27fe6c8c4fad510ac1ad Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Fri, 18 Oct 2024 14:14:17 -0400 Subject: [PATCH 16/18] don't use inputimeout --- pyproject.toml | 1 - src/pybamm/config.py | 33 ++++++++--------- tests/unit/test_config.py | 76 ++++++++++++++++++++++++++++++++------- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3eae1d9411..be07105f75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,6 @@ dependencies = [ "pandas>=1.5.0", "pooch>=1.8.1", "posthog", - "inputimeout", ] [project.urls] diff --git a/src/pybamm/config.py b/src/pybamm/config.py index 222b1d8da2..ee08ef6774 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -3,7 +3,8 @@ import platformdirs from pathlib import Path import pybamm -from inputimeout import inputimeout, TimeoutOccurred +import select +import sys def is_running_tests(): # pragma: no cover @@ -48,7 +49,7 @@ def ask_user_opt_in(timeout=10): Parameters ---------- - timeout: float, optional + timeout : float, optional The timeout for the user to respond to the prompt. Default is 10 seconds. Returns @@ -67,25 +68,21 @@ def ask_user_opt_in(timeout=10): ) while True: - try: - user_input = ( - inputimeout( - prompt="Do you want to enable telemetry? (Y/n): ", timeout=timeout - ) - .strip() - .lower() - ) - except TimeoutOccurred: + print("Do you want to enable telemetry? (Y/n): ", end="", flush=True) + + rlist, _, _ = select.select([sys.stdin], [], [], timeout) + if rlist: + user_input = sys.stdin.readline().strip().lower() + if user_input in ["yes", "y", ""]: + return True + elif user_input in ["no", "n"]: + return False + else: + print("Invalid input. Please enter 'yes/y' for yes or 'no/n' for no.") + else: print("\nTimeout reached. Defaulting to not enabling telemetry.") return False - if user_input in ["yes", "y", ""]: - return True - elif user_input in ["no", "n"]: - return False - else: - print("\nInvalid input. Please enter 'yes'/'y' or 'no'/'n'.") - def generate(): # pragma: no cover if is_running_tests(): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 6e262fb011..65d42bf168 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -1,5 +1,6 @@ import pytest -from inputimeout import TimeoutOccurred +import select +import sys import pybamm import uuid @@ -33,24 +34,73 @@ def test_write_read_uuid(self, tmp_path, write_opt_in): assert config_dict["enable_telemetry"] is False @pytest.mark.parametrize("user_opted_in, user_input", [(True, "y"), (False, "n")]) - def test_ask_user_opt_in(self, monkeypatch, user_opted_in, user_input): - # Mock the inputimeout function to return invalid input first, then valid input - inputs = iter(["invalid", user_input]) - monkeypatch.setattr( - "pybamm.config.inputimeout", lambda prompt, timeout: next(inputs) - ) + def test_ask_user_opt_in(self, monkeypatch, capsys, user_opted_in, user_input): + # Mock select.select to simulate user input + def mock_select(*args, **kwargs): + return [sys.stdin], [], [] + + monkeypatch.setattr(select, "select", mock_select) + + # Mock sys.stdin.readline to return the desired input + monkeypatch.setattr(sys.stdin, "readline", lambda: user_input + "\n") # Call the function to ask the user if they want to opt in opt_in = pybamm.config.ask_user_opt_in() + + # Check the result assert opt_in is user_opted_in - def test_ask_user_opt_in_timeout(self, monkeypatch): - # Mock the inputimeout function to raise a TimeoutOccurred exception - def mock_inputimeout(*args, **kwargs): - raise TimeoutOccurred + # Check that the prompt was printed + captured = capsys.readouterr() + assert "Do you want to enable telemetry? (Y/n):" in captured.out + + def test_ask_user_opt_in_invalid_input(self, monkeypatch, capsys): + # Mock select.select to simulate user input and then timeout + def mock_select(*args, **kwargs): + nonlocal call_count + if call_count == 0: + call_count += 1 + return [sys.stdin], [], [] + else: + return [], [], [] - monkeypatch.setattr("pybamm.config.inputimeout", mock_inputimeout) + monkeypatch.setattr(select, "select", mock_select) + + # Mock sys.stdin.readline to return invalid input + monkeypatch.setattr(sys.stdin, "readline", lambda: "invalid\n") + + # Initialize call count + call_count = 0 # Call the function to ask the user if they want to opt in - opt_in = pybamm.config.ask_user_opt_in() + opt_in = pybamm.config.ask_user_opt_in(timeout=1) + + # Check the result (should be False for timeout after invalid input) assert opt_in is False + + # Check that the prompt, invalid input message, and timeout message were printed + captured = capsys.readouterr() + assert "Do you want to enable telemetry? (Y/n):" in captured.out + assert ( + "Invalid input. Please enter 'yes/y' for yes or 'no/n' for no." + in captured.out + ) + assert "Timeout reached. Defaulting to not enabling telemetry." in captured.out + + def test_ask_user_opt_in_timeout(self, monkeypatch, capsys): + # Mock select.select to simulate a timeout + def mock_select(*args, **kwargs): + return [], [], [] + + monkeypatch.setattr(select, "select", mock_select) + + # Call the function to ask the user if they want to opt in + opt_in = pybamm.config.ask_user_opt_in(timeout=1) + + # Check the result (should be False for timeout) + assert opt_in is False + + # Check that the prompt and timeout message were printed + captured = capsys.readouterr() + assert "Do you want to enable telemetry? (Y/n):" in captured.out + assert "Timeout reached. Defaulting to not enabling telemetry." in captured.out From 1bf5b897fc6ebeee1084ef5883641f45ede36d12 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Mon, 28 Oct 2024 14:06:29 -0400 Subject: [PATCH 17/18] add more tests for config --- docs/source/user_guide/installation/index.rst | 1 + noxfile.py | 1 + src/pybamm/__init__.py | 4 +- src/pybamm/config.py | 57 +++++++++++++------ tests/unit/test_config.py | 51 +++++++++++++++++ 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/docs/source/user_guide/installation/index.rst b/docs/source/user_guide/installation/index.rst index 18e62c5dfa..9225f1ee98 100644 --- a/docs/source/user_guide/installation/index.rst +++ b/docs/source/user_guide/installation/index.rst @@ -71,6 +71,7 @@ Package Minimum supp `typing-extensions `__ 4.10.0 `pandas `__ 1.5.0 `pooch `__ 1.8.1 +`posthog `__ 3.6.5 =================================================================== ========================== .. _install.optional_dependencies: diff --git a/noxfile.py b/noxfile.py index 5ab32f463f..d65812b8ed 100644 --- a/noxfile.py +++ b/noxfile.py @@ -49,6 +49,7 @@ def set_iree_state(): "IREE_INDEX_URL": os.getenv( "IREE_INDEX_URL", "https://iree.dev/pip-release-links.html" ), + "PYBAMM_DISABLE_TELEMETRY": "true", } VENV_DIR = Path("./venv").resolve() diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py index c53d1a0e40..3eb267860a 100644 --- a/src/pybamm/__init__.py +++ b/src/pybamm/__init__.py @@ -203,7 +203,8 @@ import os import pathlib import sysconfig -os.environ["CASADIPATH"] = str(pathlib.Path(sysconfig.get_path('purelib')) / 'casadi') + +os.environ["CASADIPATH"] = str(pathlib.Path(sysconfig.get_path("purelib")) / "casadi") __all__ = [ "batch_study", @@ -232,5 +233,4 @@ "pybamm_data", ] - pybamm.config.generate() diff --git a/src/pybamm/config.py b/src/pybamm/config.py index ee08ef6774..abe775ec97 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -3,8 +3,9 @@ import platformdirs from pathlib import Path import pybamm -import select import sys +import threading +import time def is_running_tests(): # pragma: no cover @@ -14,8 +15,6 @@ def is_running_tests(): # pragma: no cover Returns: bool: True if running tests or building docs, False otherwise. """ - import sys - # Check if pytest or unittest is running if any( test_module in sys.modules for test_module in ["pytest", "unittest", "nose"] @@ -33,11 +32,14 @@ def is_running_tests(): # pragma: no cover # Check for common test runner names in command-line arguments test_runners = ["pytest", "unittest", "nose", "trial", "nox", "tox"] - if any(runner in sys.argv[0].lower() for runner in test_runners): + if any(runner in arg.lower() for arg in sys.argv for runner in test_runners): return True # Check if building docs with Sphinx - if "sphinx" in sys.modules: + if any(mod == "sphinx" or mod.startswith("sphinx.") for mod in sys.modules): + print( + f"Found Sphinx module: {[mod for mod in sys.modules if mod.startswith('sphinx')]}" + ) return True return False @@ -67,24 +69,47 @@ def ask_user_opt_in(timeout=10): "For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry" ) + def get_input(): + try: + user_input = ( + input("Do you want to enable telemetry? (Y/n): ").strip().lower() + ) + answer.append(user_input) + except Exception: + # Handle any input errors + pass + + time_start = time.time() + while True: - print("Do you want to enable telemetry? (Y/n): ", end="", flush=True) + if time.time() - time_start > timeout: + print("\nTimeout reached. Defaulting to not enabling telemetry.") + return False + + answer = [] + # Create and start input thread + input_thread = threading.Thread(target=get_input) + input_thread.daemon = True + input_thread.start() + + # Wait for either timeout or input + input_thread.join(timeout) - rlist, _, _ = select.select([sys.stdin], [], [], timeout) - if rlist: - user_input = sys.stdin.readline().strip().lower() - if user_input in ["yes", "y", ""]: + if answer: + if answer[0] in ["yes", "y", ""]: + print("\nTelemetry enabled.\n") return True - elif user_input in ["no", "n"]: + elif answer[0] in ["no", "n"]: + print("\nTelemetry disabled.\n") return False else: - print("Invalid input. Please enter 'yes/y' for yes or 'no/n' for no.") + print("\nInvalid input. Please enter 'yes/y' for yes or 'no/n' for no.") else: print("\nTimeout reached. Defaulting to not enabling telemetry.") return False -def generate(): # pragma: no cover +def generate(): if is_running_tests(): return @@ -101,7 +126,7 @@ def generate(): # pragma: no cover pybamm.telemetry.capture("user-opted-in") -def read(): # pragma: no cover +def read(): config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml" return read_uuid_from_file(config_file) @@ -121,7 +146,7 @@ def write_uuid_to_file(config_file, opt_in): def read_uuid_from_file(config_file): # Check if the config file exists - if not config_file.exists(): # pragma: no cover + if not config_file.exists(): return None # Read the UUID from the config file @@ -134,5 +159,5 @@ def read_uuid_from_file(config_file): config = yaml.safe_load(content) return config["pybamm"] - except (yaml.YAMLError, ValueError): # pragma: no cover + except (yaml.YAMLError, ValueError): return None diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 65d42bf168..62906b348d 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -4,6 +4,8 @@ import pybamm import uuid +from pathlib import Path +import platformdirs class TestConfig: @@ -104,3 +106,52 @@ def mock_select(*args, **kwargs): captured = capsys.readouterr() assert "Do you want to enable telemetry? (Y/n):" in captured.out assert "Timeout reached. Defaulting to not enabling telemetry." in captured.out + + def test_generate_and_read(self, monkeypatch, tmp_path): + # Mock is_running_tests to return False + monkeypatch.setattr(pybamm.config, "is_running_tests", lambda: False) + + # Mock ask_user_opt_in to return True + monkeypatch.setattr(pybamm.config, "ask_user_opt_in", lambda: True) + + # Mock telemetry capture + capture_called = False + + def mock_capture(event): + nonlocal capture_called + assert event == "user-opted-in" + capture_called = True + + monkeypatch.setattr(pybamm.telemetry, "capture", mock_capture) + + # Mock config directory + monkeypatch.setattr(platformdirs, "user_config_dir", lambda x: str(tmp_path)) + + # Test generate() creates new config + pybamm.config.generate() + + # Verify config was created + config = pybamm.config.read() + assert config is not None + assert config["enable_telemetry"] is True + assert "uuid" in config + assert capture_called is True + + # Test generate() does nothing if config exists + capture_called = False + pybamm.config.generate() + assert capture_called is False + + def test_read_uuid_from_file_no_file(self): + config_dict = pybamm.config.read_uuid_from_file(Path("nonexistent_file.yml")) + assert config_dict is None + + def test_read_uuid_from_file_invalid_yaml(self, tmp_path): + # Create a temporary directory and file with invalid YAML content + invalid_yaml = tmp_path / "invalid_yaml.yml" + with open(invalid_yaml, "w") as f: + f.write("invalid: yaml: content:") + + config_dict = pybamm.config.read_uuid_from_file(invalid_yaml) + + assert config_dict is None From 01832509ebb81b9be00ce0deecf1b34eeadc1e45 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 7 Nov 2024 13:35:45 -0500 Subject: [PATCH 18/18] coverage --- src/pybamm/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pybamm/config.py b/src/pybamm/config.py index abe775ec97..ba7171e5d2 100644 --- a/src/pybamm/config.py +++ b/src/pybamm/config.py @@ -69,7 +69,7 @@ def ask_user_opt_in(timeout=10): "For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry" ) - def get_input(): + def get_input(): # pragma: no cover try: user_input = ( input("Do you want to enable telemetry? (Y/n): ").strip().lower()