From 8709969884824db550fb107dd1e2de75dce86bbc Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 04:45:32 +0530
Subject: [PATCH 01/19] Make `is_notebook` a utility function
---
src/pybamm/__init__.py | 4 ++--
src/pybamm/simulation.py | 20 +-------------------
src/pybamm/util.py | 18 ++++++++++++++++++
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py
index b466c3896b..5630ae8526 100644
--- a/src/pybamm/__init__.py
+++ b/src/pybamm/__init__.py
@@ -4,10 +4,10 @@
demote_expressions_to_32bit = False
# Utility classes and methods
-from .util import root_dir
from .util import Timer, TimerTime, FuzzyDict
from .util import (
root_dir,
+ is_notebook,
load,
is_constant_and_can_evaluate,
)
@@ -190,7 +190,7 @@
from .plotting.dynamic_plot import dynamic_plot
# Simulation
-from .simulation import Simulation, load_sim, is_notebook
+from .simulation import Simulation, load_sim
# Batch Study
from .batch_study import BatchStudy
diff --git a/src/pybamm/simulation.py b/src/pybamm/simulation.py
index 75799d6334..e555e27683 100644
--- a/src/pybamm/simulation.py
+++ b/src/pybamm/simulation.py
@@ -16,24 +16,6 @@
from pybamm.expression_tree.operations.serialise import Serialise
-def is_notebook():
- try:
- shell = get_ipython().__class__.__name__
- if shell == "ZMQInteractiveShell": # pragma: no cover
- # Jupyter notebook or qtconsole
- cfg = get_ipython().config
- nb = len(cfg["InteractiveShell"].keys()) == 0
- return nb
- elif shell == "TerminalInteractiveShell": # pragma: no cover
- return False # Terminal running IPython
- elif shell == "Shell": # pragma: no cover
- return True # Google Colab notebook
- else: # pragma: no cover
- return False # Other type (?)
- except NameError:
- return False # Probably standard Python interpreter
-
-
class Simulation:
"""A Simulation class for easy building and running of PyBaMM simulations.
@@ -141,7 +123,7 @@ def __init__(
self._set_random_seed()
# ignore runtime warnings in notebooks
- if is_notebook(): # pragma: no cover
+ if pybamm.is_notebook(): # pragma: no cover
import warnings
warnings.filterwarnings("ignore")
diff --git a/src/pybamm/util.py b/src/pybamm/util.py
index 5b10b23fcb..c0c669fd9d 100644
--- a/src/pybamm/util.py
+++ b/src/pybamm/util.py
@@ -22,6 +22,24 @@ def root_dir():
return str(pathlib.Path(pybamm.__path__[0]).parent.parent)
+def is_notebook():
+ try:
+ shell = get_ipython().__class__.__name__
+ if shell == "ZMQInteractiveShell": # pragma: no cover
+ # Jupyter notebook or qtconsole
+ cfg = get_ipython().config
+ nb = len(cfg["InteractiveShell"].keys()) == 0
+ return nb
+ elif shell == "TerminalInteractiveShell": # pragma: no cover
+ return False # Terminal running IPython
+ elif shell == "Shell": # pragma: no cover
+ return True # Google Colab notebook
+ else: # pragma: no cover
+ return False # Other type (?)
+ except NameError:
+ return False # Probably standard Python interpreter
+
+
def get_git_commit_info():
"""
Get the git commit info for the current PyBaMM version, e.g. v22.8-39-gb25ce8c41
From 6fccf68c9e84b43588249b465b7835d678760991 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 04:45:50 +0530
Subject: [PATCH 02/19] Fix import order
---
src/pybamm/config.py | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index ba7171e5d2..f4641e1da7 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -1,11 +1,14 @@
-import uuid
import os
-import platformdirs
-from pathlib import Path
-import pybamm
import sys
-import threading
import time
+import uuid
+
+from pathlib import Path
+
+import pybamm
+import platformdirs
+
+from pybamm.util import is_notebook
def is_running_tests(): # pragma: no cover
From 9348522df3b456ad8ede3e5ae6864a519cec9e19 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 04:57:13 +0530
Subject: [PATCH 03/19] Add cross-platform input/timeout retrieval
---
src/pybamm/config.py | 224 +++++++++++++++++++++++++++++++++----------
1 file changed, 175 insertions(+), 49 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index f4641e1da7..d0df860996 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -48,20 +48,166 @@ def is_running_tests(): # pragma: no cover
return False
-def ask_user_opt_in(timeout=10):
+def get_input_or_timeout(timeout): # pragma: no cover
"""
- Ask the user if they want to opt in to telemetry.
+ Cross-platform input with timeout, using various methods depending on the
+ environment. Works in Jupyter notebooks, Windows, and Unix-like systems.
+
+ Returns:
+
+ Args:
+ timeout (float): Timeout in seconds
+
+ Returns:
+ tuple: A tuple containing:
+ - str: The user input if received before the timeout, or None if the timeout was reached.
+ - bool: True if the timeout was reached, False otherwise.
+ """
+ # Check for telemetry disable flag
+ if os.getenv("PYBAMM_DISABLE_TELEMETRY", "false").lower() != "false":
+ return None, True
+
+ if not (sys.stdin.isatty() or is_notebook()):
+ return None, True
+
+ # 1. special handling for Jupyter notebooks
+ if is_notebook():
+ try:
+ from ipywidgets import widgets
+ from IPython.display import display, clear_output
+
+ # Create buttons for yes/no
+ yes_button = widgets.Button(description="Yes")
+ no_button = widgets.Button(description="No")
+ output = widgets.Output()
+
+ # Variable to store the result
+ result = {"value": None, "set": False}
+
+ def on_yes_clicked(b):
+ with output:
+ result["value"] = "yes"
+ result["set"] = True
+ clear_output()
+ print("Telemetry enabled.")
+
+ def on_no_clicked(b):
+ with output:
+ result["value"] = "no"
+ result["set"] = True
+ clear_output()
+ print("Telemetry disabled.")
+
+ yes_button.on_click(on_yes_clicked)
+ no_button.on_click(on_no_clicked)
+
+ # Display the buttons
+ print("Do you want to enable telemetry?")
+ display(widgets.HBox([yes_button, no_button]))
+ display(output)
+
+ # Wait for button click or timeout
+ start_time = time.time()
+ while not result["set"] and (time.time() - start_time < timeout):
+ time.sleep(0.05)
+
+ if not result["set"]:
+ with output:
+ clear_output()
+ print("Timeout reached or negative input. Defaulting to not enabling telemetry.")
+ return None, True
+
+ return result["value"], False
+
+ except Exception:
+ # Fallback to regular input for Jupyter environments where widgets
+ # aren't available. This should be quite rare at this point but is
+ # included for completeness.
+ try:
+ from IPython.display import clear_output
+
+ user_input = input("Do you want to enable telemetry? (Y/n): ")
+ clear_output()
+ return user_input, False
+ except Exception:
+ return None, True
+
+ # 2. Windows-specific handling
+ if sys.platform == "win32":
+ try:
+ import msvcrt
+
+ start_time = time.time()
+ input_chars = []
+ sys.stdout.write("Do you want to enable telemetry? (Y/n): ")
+ sys.stdout.flush()
+
+ while time.time() - start_time < timeout:
+ if msvcrt.kbhit():
+ char = msvcrt.getwche()
+ if char in ("\r", "\n"):
+ sys.stdout.write("\n")
+ return "".join(input_chars), False
+ input_chars.append(char)
+ time.sleep(0.1)
+ return None, True
+ except Exception:
+ return None, True
+ # POSIX-like systems will need to use termios
+ else:
+ try:
+ import termios
+ import tty
+ import select
+
+ # Save terminal settings for later
+ old_settings = termios.tcgetattr(sys.stdin)
+ try:
+ # Set terminal to raw mode
+ tty.setraw(sys.stdin.fileno())
+
+ sys.stdout.write("Do you want to enable telemetry? (Y/n): ")
+ sys.stdout.flush()
+
+ input_chars = []
+ start_time = time.time()
+
+ while time.time() - start_time < timeout:
+ rlist, _, _ = select.select([sys.stdin], [], [], 0.1)
+ if rlist:
+ char = sys.stdin.read(1)
+ if char in ("\r", "\n"):
+ sys.stdout.write("\n")
+ return "".join(input_chars), False
+ input_chars.append(char)
+ sys.stdout.write(char)
+ sys.stdout.flush()
+ return None, True
+
+ finally:
+ # Restore saved terminal settings
+ termios.tcsetattr(sys.stdin, termios.TCSADRAIN, old_settings)
+ sys.stdout.write("\n")
+ sys.stdout.flush()
+
+ except Exception:
+ return None, True
+
+ return None, True
- Parameters
- ----------
- timeout : float, optional
- The timeout for the user to respond to the prompt. Default is 10 seconds.
- Returns
- -------
- bool
- True if the user opts in, False otherwise.
+def ask_user_opt_in(timeout=10): # pragma: no cover
"""
+ Ask the user if they want to opt in to telemetry.
+ """
+ # Check for telemetry disable flag first
+ if os.getenv("PYBAMM_DISABLE_TELEMETRY", "false").lower() != "false":
+ return False
+
+ # Skip telemetry prompt in non-interactive environments
+ if not (sys.stdin.isatty() or is_notebook()):
+ False
+
print(
"PyBaMM can collect usage data and send it to the PyBaMM team to "
"help us improve the software.\n"
@@ -72,44 +218,24 @@ 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(): # pragma: no cover
- 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:
- 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)
-
- if answer:
- if answer[0] in ["yes", "y", ""]:
- print("\nTelemetry enabled.\n")
- return True
- elif answer[0] in ["no", "n"]:
- print("\nTelemetry disabled.\n")
- return False
- else:
- 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
+ user_input, timed_out = get_input_or_timeout(timeout)
+
+ if timed_out:
+ print("\nTimeout reached. Defaulting to not enabling telemetry.")
+ return False
+
+ if user_input is None or not user_input: # Empty input should mean a yes
+ print("\nTelemetry enabled.\n")
+ return True
+ elif user_input.lower() in ["y", "yes"]:
+ print("\nTelemetry enabled.\n")
+ return True
+ elif user_input.lower() in ["n", "no"]:
+ print("\nTelemetry disabled.\n")
+ return False
+ else:
+ print("\nInvalid input. Defaulting to not enabling telemetry.")
+ return False
def generate():
@@ -121,7 +247,7 @@ def generate():
return
# Ask the user if they want to opt in to telemetry
- opt_in = ask_user_opt_in()
+ opt_in = ask_user_opt_in(timeout=10)
config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml"
write_uuid_to_file(config_file, opt_in)
From c89ee9c517ee6561ee8b9c080b2fb8434ae621ec Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 05:05:30 +0530
Subject: [PATCH 04/19] Mark missing dependency on `platformdirs`
---
docs/source/user_guide/installation/index.rst | 1 +
pyproject.toml | 1 +
2 files changed, 2 insertions(+)
diff --git a/docs/source/user_guide/installation/index.rst b/docs/source/user_guide/installation/index.rst
index 9225f1ee98..d475a1b308 100644
--- a/docs/source/user_guide/installation/index.rst
+++ b/docs/source/user_guide/installation/index.rst
@@ -72,6 +72,7 @@ Package Minimum supp
`pandas `__ 1.5.0
`pooch `__ 1.8.1
`posthog `__ 3.6.5
+`platformdirs `__ 4.0.0
=================================================================== ==========================
.. _install.optional_dependencies:
diff --git a/pyproject.toml b/pyproject.toml
index be07105f75..e0d20d15f6 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -45,6 +45,7 @@ dependencies = [
"pandas>=1.5.0",
"pooch>=1.8.1",
"posthog",
+ "platformdirs",
]
[project.urls]
From b6755029f851d8693463e97a45f571930a6b7237 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 05:32:48 +0530
Subject: [PATCH 05/19] Fix `pytest` invocation
---
noxfile.py | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/noxfile.py b/noxfile.py
index d65812b8ed..86aaa5159b 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -254,9 +254,10 @@ def run_tests(session):
session.install("setuptools", silent=False)
session.install("-e", ".[all,dev,jax]", silent=False)
specific_test_files = session.posargs if session.posargs else []
- session.run(
- "python", "-m", "pytest", *specific_test_files, "-m", "unit or integration"
- )
+ if specific_test_files:
+ session.run("python", "-m", "pytest", *specific_test_files)
+ else:
+ session.run("python", "-m", "pytest", "-m", "unit or integration")
@nox.session(name="docs")
From e8680dfbb7b5501c830ae431d8216beb78808031 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 05:52:36 +0530
Subject: [PATCH 06/19] Fix coverage and return value for prompt
---
src/pybamm/config.py | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index d0df860996..ab33c17ad5 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -71,7 +71,7 @@ def get_input_or_timeout(timeout): # pragma: no cover
return None, True
# 1. special handling for Jupyter notebooks
- if is_notebook():
+ if is_notebook(): # pragma: no cover
try:
from ipywidgets import widgets
from IPython.display import display, clear_output
@@ -114,12 +114,14 @@ def on_no_clicked(b):
if not result["set"]:
with output:
clear_output()
- print("Timeout reached or negative input. Defaulting to not enabling telemetry.")
+ print(
+ "Timeout reached or negative input. Defaulting to not enabling telemetry."
+ )
return None, True
return result["value"], False
- except Exception:
+ except Exception: # pragma: no cover
# Fallback to regular input for Jupyter environments where widgets
# aren't available. This should be quite rare at this point but is
# included for completeness.
@@ -153,8 +155,8 @@ def on_no_clicked(b):
return None, True
except Exception:
return None, True
- # POSIX-like systems will need to use termios
- else:
+ # 3. POSIX-like systems will need to use termios
+ else: # pragma: no cover
try:
import termios
import tty
@@ -190,7 +192,7 @@ def on_no_clicked(b):
sys.stdout.write("\n")
sys.stdout.flush()
- except Exception:
+ except Exception: # pragma: no cover
return None, True
return None, True
@@ -206,7 +208,7 @@ def ask_user_opt_in(timeout=10): # pragma: no cover
# Skip telemetry prompt in non-interactive environments
if not (sys.stdin.isatty() or is_notebook()):
- False
+ return False
print(
"PyBaMM can collect usage data and send it to the PyBaMM team to "
From af0de0f5bdbf7e5acd0d0866199ee4038e360c74 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 05:55:02 +0530
Subject: [PATCH 07/19] Fix tests and parametrize them
---
tests/unit/test_config.py | 182 ++++++++++++++++++--------------------
1 file changed, 87 insertions(+), 95 deletions(-)
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 62906b348d..40cef6b222 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -1,6 +1,6 @@
import pytest
-import select
import sys
+import os
import pybamm
import uuid
@@ -35,86 +35,60 @@ def test_write_read_uuid(self, tmp_path, write_opt_in):
else:
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, 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
-
- # 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(select, "select", mock_select)
+ @pytest.mark.parametrize(
+ "user_input,expected_output,expected_message",
+ [
+ ("y", True, "Telemetry enabled"),
+ ("n", False, "Telemetry disabled"),
+ ("x", False, "Invalid input"),
+ (None, False, "Timeout reached"),
+ ],
+ )
+ def test_ask_user_opt_in_scenarios(
+ self, monkeypatch, capsys, user_input, expected_output, expected_message
+ ):
+ # mock is_running_tests to return False. This is done
+ # temporarily here in order to prevent an early return.
+ monkeypatch.setattr(pybamm.config, "is_running_tests", lambda: False)
+ monkeypatch.setattr(os, "getenv", lambda x, y: "false")
+ monkeypatch.setattr(sys.stdin, "isatty", lambda: True)
+ monkeypatch.setattr(pybamm.util, "is_notebook", lambda: False)
- # Mock sys.stdin.readline to return invalid input
- monkeypatch.setattr(sys.stdin, "readline", lambda: "invalid\n")
+ # Mock get_input_or_timeout based on scenario
+ def mock_get_input(timeout):
+ print("Do you want to enable telemetry? (Y/n): ", end="")
+ return user_input, user_input is None
- # Initialize call count
- call_count = 0
+ monkeypatch.setattr(pybamm.config, "get_input_or_timeout", mock_get_input)
- # 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 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)
+ assert "Do you want to enable telemetry?" in captured.out
+ assert "PyBaMM can collect usage data" in captured.out
+ assert expected_message in captured.out
+ assert opt_in is expected_output
- # 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
-
- def test_generate_and_read(self, monkeypatch, tmp_path):
+ @pytest.mark.parametrize(
+ "test_scenario",
+ [
+ "first_generation", # Test first-time config generation
+ "config_exists", # Test when config already exists
+ ],
+ )
+ def test_generate_and_read(self, monkeypatch, tmp_path, test_scenario, timeout=2):
# 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)
+ def mock_ask_user_opt_in(timeout=10):
+ return True
- # Mock telemetry capture
+ monkeypatch.setattr(pybamm.config, "ask_user_opt_in", mock_ask_user_opt_in)
+
+ # Track if capture was called
capture_called = False
def mock_capture(event):
@@ -127,31 +101,49 @@ def mock_capture(event):
# 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
+ if test_scenario == "first_generation":
+ # Test first-time generation
+ 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
+ ) # Should not ask for capturing telemetry when config exists
+
+ else: # config_exists case
+ # First create a config
+ pybamm.config.generate()
+ capture_called = False # Reset the flag
+
+ # Now test that generating again does nothing
+ pybamm.config.generate()
+ assert (
+ capture_called is False
+ ) # Should not ask for capturing telemetry when config exists
+
+ @pytest.mark.parametrize(
+ "file_scenario,expected_output",
+ [
+ ("nonexistent", None),
+ ("invalid_yaml", None),
+ ],
+ )
+ def test_read_uuid_from_file_scenarios(
+ self, tmp_path, file_scenario, expected_output
+ ):
+ if file_scenario == "nonexistent":
+ config_dict = pybamm.config.read_uuid_from_file(
+ Path("nonexistent_file.yml")
+ )
+ else: # invalid_yaml
+ # 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 expected_output
From 925cbd6bf89749509394dcb19117c7626899b3b5 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 05:56:16 +0530
Subject: [PATCH 08/19] Append to CHANGELOG entry
---
CHANGELOG.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eee7de05d7..1d2e6a9e74 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,7 @@
- Adds an option "voltage as a state" that can be "false" (default) or "true". If "true" adds an explicit algebraic equation for the voltage. ([#4507](https://github.com/pybamm-team/PyBaMM/pull/4507))
- Improved `QuickPlot` accuracy for simulations with Hermite interpolation. ([#4483](https://github.com/pybamm-team/PyBaMM/pull/4483))
- Added Hermite interpolation to the (`IDAKLUSolver`) that improves the accuracy and performance of post-processing variables. ([#4464](https://github.com/pybamm-team/PyBaMM/pull/4464))
-- 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 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), [#4583](https://github.com/pybamm-team/PyBaMM/pull/4583))
- Added `BasicDFN` model for sodium-ion batteries ([#4451](https://github.com/pybamm-team/PyBaMM/pull/4451))
- Added sensitivity calculation support for `pybamm.Simulation` and `pybamm.Experiment` ([#4415](https://github.com/pybamm-team/PyBaMM/pull/4415))
- Added OpenMP parallelization to IDAKLU solver for lists of input parameters ([#4449](https://github.com/pybamm-team/PyBaMM/pull/4449))
From d8d37ed59bb8152cc187c329d1f7860e3edfc9ba Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
<66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Wed, 13 Nov 2024 00:32:51 +0000
Subject: [PATCH 09/19] style: pre-commit fixes
---
tests/unit/test_config.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 40cef6b222..22ed9522a1 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -35,7 +35,6 @@ def test_write_read_uuid(self, tmp_path, write_opt_in):
else:
assert config_dict["enable_telemetry"] is False
-
@pytest.mark.parametrize(
"user_input,expected_output,expected_message",
[
@@ -70,7 +69,6 @@ def mock_get_input(timeout):
assert expected_message in captured.out
assert opt_in is expected_output
-
@pytest.mark.parametrize(
"test_scenario",
[
From fbb623e3a94c46a3a75b31d409a550534bbb9461 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 19:46:24 +0530
Subject: [PATCH 10/19] Something simpler for Jupyter notebooks
---
src/pybamm/config.py | 77 ++++++++++----------------------------------
1 file changed, 17 insertions(+), 60 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index ab33c17ad5..65a6bf2115 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -53,8 +53,6 @@ def get_input_or_timeout(timeout): # pragma: no cover
Cross-platform input with timeout, using various methods depending on the
environment. Works in Jupyter notebooks, Windows, and Unix-like systems.
- Returns:
-
Args:
timeout (float): Timeout in seconds
@@ -70,69 +68,27 @@ def get_input_or_timeout(timeout): # pragma: no cover
if not (sys.stdin.isatty() or is_notebook()):
return None, True
- # 1. special handling for Jupyter notebooks
+ # 1. Handling for Jupyter notebooks. This is a simplified
+ # implementation in comparison to widgets because notebooks
+ # are usually run interactively, and we don't need to worry
+ # about the event loop. The input is disabled when running
+ # tests due to the environment variable set.
if is_notebook(): # pragma: no cover
try:
- from ipywidgets import widgets
- from IPython.display import display, clear_output
-
- # Create buttons for yes/no
- yes_button = widgets.Button(description="Yes")
- no_button = widgets.Button(description="No")
- output = widgets.Output()
-
- # Variable to store the result
- result = {"value": None, "set": False}
-
- def on_yes_clicked(b):
- with output:
- result["value"] = "yes"
- result["set"] = True
- clear_output()
- print("Telemetry enabled.")
-
- def on_no_clicked(b):
- with output:
- result["value"] = "no"
- result["set"] = True
- clear_output()
- print("Telemetry disabled.")
-
- yes_button.on_click(on_yes_clicked)
- no_button.on_click(on_no_clicked)
-
- # Display the buttons
- print("Do you want to enable telemetry?")
- display(widgets.HBox([yes_button, no_button]))
- display(output)
-
- # Wait for button click or timeout
- start_time = time.time()
- while not result["set"] and (time.time() - start_time < timeout):
- time.sleep(0.05)
-
- if not result["set"]:
- with output:
- clear_output()
- print(
- "Timeout reached or negative input. Defaulting to not enabling telemetry."
- )
- return None, True
+ from IPython.display import clear_output
- return result["value"], False
+ user_input = input("Do you want to enable telemetry? (Y/n): ")
+ clear_output()
- except Exception: # pragma: no cover
- # Fallback to regular input for Jupyter environments where widgets
- # aren't available. This should be quite rare at this point but is
- # included for completeness.
- try:
- from IPython.display import clear_output
+ if user_input.lower() in ["y", "yes", ""]:
+ print("Telemetry enabled.")
+ elif user_input.lower() in ["n", "no"]:
+ print("Telemetry disabled.")
- user_input = input("Do you want to enable telemetry? (Y/n): ")
- clear_output()
- return user_input, False
- except Exception:
- return None, True
+ return user_input, False
+
+ except Exception: # pragma: no cover
+ return None, True
# 2. Windows-specific handling
if sys.platform == "win32":
@@ -155,6 +111,7 @@ def on_no_clicked(b):
return None, True
except Exception:
return None, True
+
# 3. POSIX-like systems will need to use termios
else: # pragma: no cover
try:
From 0826afef119f7748f51b7c572089f13cac676ec6 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 19:46:51 +0530
Subject: [PATCH 11/19] Remove duplicate outputs from print statements
---
src/pybamm/config.py | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index 65a6bf2115..6ec16f69d9 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -80,11 +80,6 @@ def get_input_or_timeout(timeout): # pragma: no cover
user_input = input("Do you want to enable telemetry? (Y/n): ")
clear_output()
- if user_input.lower() in ["y", "yes", ""]:
- print("Telemetry enabled.")
- elif user_input.lower() in ["n", "no"]:
- print("Telemetry disabled.")
-
return user_input, False
except Exception: # pragma: no cover
From 4f8b48cdff2cd37f443a4f91bfbd96616e70b089 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 20:07:51 +0530
Subject: [PATCH 12/19] Simplify command for test invocation
---
noxfile.py | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/noxfile.py b/noxfile.py
index 86aaa5159b..bd38528778 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -253,11 +253,12 @@ def run_tests(session):
set_environment_variables(PYBAMM_ENV, session=session)
session.install("setuptools", silent=False)
session.install("-e", ".[all,dev,jax]", silent=False)
- specific_test_files = session.posargs if session.posargs else []
- if specific_test_files:
- session.run("python", "-m", "pytest", *specific_test_files)
- else:
- session.run("python", "-m", "pytest", "-m", "unit or integration")
+ session.run(
+ "python",
+ "-m",
+ "pytest",
+ *(session.posargs if session.posargs else ["-m", "unit or integration"]),
+ )
@nox.session(name="docs")
From 91379bd46945af73cb08ba59a2c91acac5402561 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 21:59:52 +0530
Subject: [PATCH 13/19] Return either input as a string, or None
---
src/pybamm/config.py | 39 ++++++++++++++++++++++-----------------
tests/unit/test_config.py | 2 +-
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index 6ec16f69d9..a7d5c09488 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -57,16 +57,21 @@ def get_input_or_timeout(timeout): # pragma: no cover
timeout (float): Timeout in seconds
Returns:
- tuple: A tuple containing:
- - str: The user input if received before the timeout, or None if the timeout was reached.
- - bool: True if the timeout was reached, False otherwise.
+ str | None: The user input if received before the timeout, or None if:
+ - The timeout was reached
+ - Telemetry is disabled via environment variable
+ - Running in a non-interactive environment
+ - An error occurred
+
+ The caller can distinguish between an empty response (user just pressed Enter)
+ which returns '', and a timeout/error which returns None.
"""
# Check for telemetry disable flag
if os.getenv("PYBAMM_DISABLE_TELEMETRY", "false").lower() != "false":
- return None, True
+ return None
if not (sys.stdin.isatty() or is_notebook()):
- return None, True
+ return None
# 1. Handling for Jupyter notebooks. This is a simplified
# implementation in comparison to widgets because notebooks
@@ -80,10 +85,10 @@ def get_input_or_timeout(timeout): # pragma: no cover
user_input = input("Do you want to enable telemetry? (Y/n): ")
clear_output()
- return user_input, False
+ return user_input
except Exception: # pragma: no cover
- return None, True
+ return None
# 2. Windows-specific handling
if sys.platform == "win32":
@@ -100,12 +105,12 @@ def get_input_or_timeout(timeout): # pragma: no cover
char = msvcrt.getwche()
if char in ("\r", "\n"):
sys.stdout.write("\n")
- return "".join(input_chars), False
+ return "".join(input_chars)
input_chars.append(char)
time.sleep(0.1)
- return None, True
+ return None
except Exception:
- return None, True
+ return None
# 3. POSIX-like systems will need to use termios
else: # pragma: no cover
@@ -132,11 +137,11 @@ def get_input_or_timeout(timeout): # pragma: no cover
char = sys.stdin.read(1)
if char in ("\r", "\n"):
sys.stdout.write("\n")
- return "".join(input_chars), False
+ return "".join(input_chars)
input_chars.append(char)
sys.stdout.write(char)
sys.stdout.flush()
- return None, True
+ return None
finally:
# Restore saved terminal settings
@@ -145,9 +150,9 @@ def get_input_or_timeout(timeout): # pragma: no cover
sys.stdout.flush()
except Exception: # pragma: no cover
- return None, True
+ return None
- return None, True
+ return None
def ask_user_opt_in(timeout=10): # pragma: no cover
@@ -172,13 +177,13 @@ def ask_user_opt_in(timeout=10): # pragma: no cover
"For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry"
)
- user_input, timed_out = get_input_or_timeout(timeout)
+ user_input = get_input_or_timeout(timeout)
- if timed_out:
+ if user_input is None:
print("\nTimeout reached. Defaulting to not enabling telemetry.")
return False
- if user_input is None or not user_input: # Empty input should mean a yes
+ if not user_input: # Empty input should mean a yes
print("\nTelemetry enabled.\n")
return True
elif user_input.lower() in ["y", "yes"]:
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 22ed9522a1..e3500175ef 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -57,7 +57,7 @@ def test_ask_user_opt_in_scenarios(
# Mock get_input_or_timeout based on scenario
def mock_get_input(timeout):
print("Do you want to enable telemetry? (Y/n): ", end="")
- return user_input, user_input is None
+ return user_input
monkeypatch.setattr(pybamm.config, "get_input_or_timeout", mock_get_input)
From de60647585b6e58e6e6953b81eac9ce88853b328 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 22:06:41 +0530
Subject: [PATCH 14/19] Keep prompting on invalid inputs
---
src/pybamm/config.py | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index a7d5c09488..596ef33e37 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -183,18 +183,22 @@ def ask_user_opt_in(timeout=10): # pragma: no cover
print("\nTimeout reached. Defaulting to not enabling telemetry.")
return False
- if not user_input: # Empty input should mean a yes
- print("\nTelemetry enabled.\n")
- return True
- elif user_input.lower() in ["y", "yes"]:
- print("\nTelemetry enabled.\n")
- return True
- elif user_input.lower() in ["n", "no"]:
- print("\nTelemetry disabled.\n")
- return False
- else:
- print("\nInvalid input. Defaulting to not enabling telemetry.")
- return False
+ while True:
+ if user_input == "": # Empty input should mean a no
+ print("Telemetry enabled.\n")
+ return False
+ elif user_input.lower() in ["y", "yes"]:
+ print("Telemetry enabled.\n")
+ return True
+ elif user_input.lower() in ["n", "no"]:
+ print("Telemetry disabled.")
+ return False
+ else:
+ print("Invalid input. Please enter 'Y/y' for yes or 'n/N' for no:")
+ user_input = get_input_or_timeout(timeout)
+ if user_input is None:
+ print("\nTimeout reached. Defaulting to not enabling telemetry.")
+ return False
def generate():
From 083184b35925dbc1b203719ab2c8329c4b9457a9 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Wed, 13 Nov 2024 22:11:15 +0530
Subject: [PATCH 15/19] Fix tests for invalid inputs
---
tests/unit/test_config.py | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index e3500175ef..738b530ce5 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -36,16 +36,19 @@ def test_write_read_uuid(self, tmp_path, write_opt_in):
assert config_dict["enable_telemetry"] is False
@pytest.mark.parametrize(
- "user_input,expected_output,expected_message",
+ "input_sequence,expected_output,expected_messages",
[
- ("y", True, "Telemetry enabled"),
- ("n", False, "Telemetry disabled"),
- ("x", False, "Invalid input"),
- (None, False, "Timeout reached"),
+ (["y"], True, ["Telemetry enabled"]),
+ (["n"], False, ["Telemetry disabled"]),
+ ([""], False, ["Telemetry enabled"]),
+ (["x", "y"], True, ["Invalid input", "Telemetry enabled"]),
+ (["x", "n"], False, ["Invalid input", "Telemetry disabled"]),
+ (["x", None], False, ["Invalid input", "Timeout reached"]),
+ ([None], False, ["Timeout reached"]),
],
)
def test_ask_user_opt_in_scenarios(
- self, monkeypatch, capsys, user_input, expected_output, expected_message
+ self, monkeypatch, capsys, input_sequence, expected_output, expected_messages
):
# mock is_running_tests to return False. This is done
# temporarily here in order to prevent an early return.
@@ -54,10 +57,15 @@ def test_ask_user_opt_in_scenarios(
monkeypatch.setattr(sys.stdin, "isatty", lambda: True)
monkeypatch.setattr(pybamm.util, "is_notebook", lambda: False)
- # Mock get_input_or_timeout based on scenario
+ # Mock get_input_or_timeout to return sequence of inputs
+ input_iter = iter(input_sequence)
+
def mock_get_input(timeout):
print("Do you want to enable telemetry? (Y/n): ", end="")
- return user_input
+ try:
+ return next(input_iter)
+ except StopIteration:
+ return None
monkeypatch.setattr(pybamm.config, "get_input_or_timeout", mock_get_input)
@@ -66,7 +74,8 @@ def mock_get_input(timeout):
assert "Do you want to enable telemetry?" in captured.out
assert "PyBaMM can collect usage data" in captured.out
- assert expected_message in captured.out
+ for message in expected_messages:
+ assert message in captured.out
assert opt_in is expected_output
@pytest.mark.parametrize(
From 6f077e360d77f9958b5e8c5f1a3bcc23b86362d7 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Thu, 14 Nov 2024 05:21:10 +0530
Subject: [PATCH 16/19] Code review suggestions
Co-authored-by: Eric G. Kratz
---
src/pybamm/config.py | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index 596ef33e37..cd10f4bc68 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -184,10 +184,7 @@ def ask_user_opt_in(timeout=10): # pragma: no cover
return False
while True:
- if user_input == "": # Empty input should mean a no
- print("Telemetry enabled.\n")
- return False
- elif user_input.lower() in ["y", "yes"]:
+ if user_input.lower() in ["y", "yes", ""]:
print("Telemetry enabled.\n")
return True
elif user_input.lower() in ["n", "no"]:
@@ -210,7 +207,7 @@ def generate():
return
# Ask the user if they want to opt in to telemetry
- opt_in = ask_user_opt_in(timeout=10)
+ opt_in = ask_user_opt_in()
config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml"
write_uuid_to_file(config_file, opt_in)
From 7433555d2d88ba061651caa57d5dc46b44b712b8 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Thu, 14 Nov 2024 05:22:53 +0530
Subject: [PATCH 17/19] Remove duplicate checks
---
src/pybamm/config.py | 7 -------
1 file changed, 7 deletions(-)
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
index cd10f4bc68..807a23f274 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -159,13 +159,6 @@ def ask_user_opt_in(timeout=10): # pragma: no cover
"""
Ask the user if they want to opt in to telemetry.
"""
- # Check for telemetry disable flag first
- if os.getenv("PYBAMM_DISABLE_TELEMETRY", "false").lower() != "false":
- return False
-
- # Skip telemetry prompt in non-interactive environments
- if not (sys.stdin.isatty() or is_notebook()):
- return False
print(
"PyBaMM can collect usage data and send it to the PyBaMM team to "
From 57553dfc549146c128cc8abd2442e30a3e651d79 Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Thu, 14 Nov 2024 15:58:33 +0530
Subject: [PATCH 18/19] Discard previous outputs as well
---
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 807a23f274..69b5e0e6d7 100644
--- a/src/pybamm/config.py
+++ b/src/pybamm/config.py
@@ -145,7 +145,7 @@ def get_input_or_timeout(timeout): # pragma: no cover
finally:
# Restore saved terminal settings
- termios.tcsetattr(sys.stdin, termios.TCSADRAIN, old_settings)
+ termios.tcsetattr(sys.stdin, termios.TCSAFLUSH, old_settings)
sys.stdout.write("\n")
sys.stdout.flush()
From e4859dcff274d584ce31b31cb33e3217f302799f Mon Sep 17 00:00:00 2001
From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Date: Thu, 14 Nov 2024 16:05:05 +0530
Subject: [PATCH 19/19] Fix failing test
---
tests/unit/test_config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 738b530ce5..67f7ae1dec 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -40,7 +40,7 @@ def test_write_read_uuid(self, tmp_path, write_opt_in):
[
(["y"], True, ["Telemetry enabled"]),
(["n"], False, ["Telemetry disabled"]),
- ([""], False, ["Telemetry enabled"]),
+ ([""], True, ["Telemetry enabled"]),
(["x", "y"], True, ["Invalid input", "Telemetry enabled"]),
(["x", "n"], False, ["Invalid input", "Telemetry disabled"]),
(["x", None], False, ["Invalid input", "Timeout reached"]),