diff --git a/src/h5forest/bindings/hist_bindings.py b/src/h5forest/bindings/hist_bindings.py index 663620d..839dbc4 100644 --- a/src/h5forest/bindings/hist_bindings.py +++ b/src/h5forest/bindings/hist_bindings.py @@ -9,7 +9,7 @@ from prompt_toolkit.widgets import Label from h5forest.config import translate_key_label -from h5forest.dataset_prompts import prompt_for_dataset_operation +from h5forest.dataset_prompts import prompt_for_chunking_preference from h5forest.errors import error_handler from h5forest.utils import WaitIndicator @@ -28,13 +28,8 @@ def select_data(event): app.print(f"{node.path} is not a Dataset") return - def run_operation(use_chunks): - """Set histogram data after user confirmation.""" - # Set the text in the histogram area - app.hist_content.text = app.histogram_plotter.set_data_key(node) - - # Prompt user if needed, then run operation - prompt_for_dataset_operation(app, node, run_operation) + # Set the text in the histogram area directly (no prompt here) + app.hist_content.text = app.histogram_plotter.set_data_key(node) @error_handler def edit_bins(event): @@ -247,37 +242,32 @@ def plot_hist(event): app.print(f"{node.path} is not a Dataset") return - def run_operation(use_chunks): - """Set histogram data and plot after user confirmation.""" - # Set the text in the plotting area - app.hist_content.text = app.histogram_plotter.set_data_key( - node - ) - - # Compute and plot the histogram with wait indicator - with WaitIndicator(app, "Generating histogram..."): - # Compute the histogram - app.hist_content.text = app.histogram_plotter.compute_hist( - app.hist_content.text - ) - - # Get the plot - app.histogram_plotter.plot_and_show(app.hist_content.text) + # Set the text in the plotting area + app.hist_content.text = app.histogram_plotter.set_data_key(node) - # Prompt user if needed, then run operation - prompt_for_dataset_operation(app, node, run_operation) - else: - # Already have data, just plot + def do_plot(use_chunks): + """Actually perform the plot after chunking preference is set.""" # Compute and plot the histogram with wait indicator with WaitIndicator(app, "Generating histogram..."): # Compute the histogram app.hist_content.text = app.histogram_plotter.compute_hist( - app.hist_content.text + app.hist_content.text, use_chunks=use_chunks ) # Get the plot app.histogram_plotter.plot_and_show(app.hist_content.text) + # Check if we have data selected + if "data" not in app.histogram_plotter.plot_params: + app.print("Please select a dataset first (Enter)") + return + + # Get the node to check for chunking + nodes = [app.histogram_plotter.plot_params["data"]] + + # Prompt for chunking preference if needed, then plot + prompt_for_chunking_preference(app, nodes, do_plot) + @error_handler def save_hist(event): """Plot the histogram.""" @@ -291,33 +281,30 @@ def save_hist(event): app.print(f"{node.path} is not a Dataset") return - def run_operation(use_chunks): - """Set histogram data and save after user confirmation.""" - # Set the text in the plotting area - app.hist_content.text = app.histogram_plotter.set_data_key( - node - ) - - # Compute the histogram - app.hist_content.text = app.histogram_plotter.compute_hist( - app.hist_content.text - ) - - # Get the plot - app.histogram_plotter.plot_and_save(app.hist_content.text) + # Set the text in the plotting area + app.hist_content.text = app.histogram_plotter.set_data_key(node) - # Prompt user if needed, then run operation - prompt_for_dataset_operation(app, node, run_operation) - else: - # Already have data, just save + def do_save(use_chunks): + """Actually save the plot after chunking preference is set.""" # Compute the histogram app.hist_content.text = app.histogram_plotter.compute_hist( - app.hist_content.text + app.hist_content.text, use_chunks=use_chunks ) # Get the plot app.histogram_plotter.plot_and_save(app.hist_content.text) + # Check if we have data selected + if "data" not in app.histogram_plotter.plot_params: + app.print("Please select a dataset first (Enter)") + return + + # Get the node to check for chunking + nodes = [app.histogram_plotter.plot_params["data"]] + + # Prompt for chunking preference if needed, then save + prompt_for_chunking_preference(app, nodes, do_save) + @error_handler def reset_hist(event): """Reset the histogram content.""" diff --git a/src/h5forest/bindings/plot_bindings.py b/src/h5forest/bindings/plot_bindings.py index a8ddfe5..1ac24af 100644 --- a/src/h5forest/bindings/plot_bindings.py +++ b/src/h5forest/bindings/plot_bindings.py @@ -10,6 +10,7 @@ from prompt_toolkit.widgets import Label from h5forest.config import translate_key_label +from h5forest.dataset_prompts import prompt_for_chunking_preference from h5forest.errors import error_handler from h5forest.utils import WaitIndicator @@ -211,16 +212,60 @@ def edit_plot_entry_callback(): @error_handler def plot_scatter(event): """Plot and show scatter with mean in bins.""" - # Make the plot with wait indicator - with WaitIndicator(app, "Generating scatter plot..."): - app.scatter_plotter.plot_and_show(app.plot_content.text) + # Check if we have both x and y datasets selected + if ( + "x" not in app.scatter_plotter.plot_params + or "y" not in app.scatter_plotter.plot_params + ): + msg = "Please select both x-axis (x) and y-axis (y) datasets first" + app.print(msg) + return - app.default_focus() + def do_plot(use_chunks): + """Actually perform the plot after chunking preference is set.""" + # Make the plot with wait indicator + with WaitIndicator(app, "Generating scatter plot..."): + app.scatter_plotter.plot_and_show( + app.plot_content.text, use_chunks=use_chunks + ) + + app.default_focus() + + # Get the nodes to check for chunking + nodes = [ + app.scatter_plotter.plot_params["x"], + app.scatter_plotter.plot_params["y"], + ] + + # Prompt for chunking preference if needed, then plot + prompt_for_chunking_preference(app, nodes, do_plot) @error_handler def save_scatter(event): """Save the plot.""" - app.scatter_plotter.plot_and_save(app.plot_content.text) + # Check if we have both x and y datasets selected + if ( + "x" not in app.scatter_plotter.plot_params + or "y" not in app.scatter_plotter.plot_params + ): + msg = "Please select both x-axis (x) and y-axis (y) datasets first" + app.print(msg) + return + + def do_save(use_chunks): + """Actually save the plot after chunking preference is set.""" + app.scatter_plotter.plot_and_save( + app.plot_content.text, use_chunks=use_chunks + ) + + # Get the nodes to check for chunking + nodes = [ + app.scatter_plotter.plot_params["x"], + app.scatter_plotter.plot_params["y"], + ] + + # Prompt for chunking preference if needed, then save + prompt_for_chunking_preference(app, nodes, do_save) @error_handler def reset(event): diff --git a/src/h5forest/dataset_prompts.py b/src/h5forest/dataset_prompts.py index b9d6255..0448374 100644 --- a/src/h5forest/dataset_prompts.py +++ b/src/h5forest/dataset_prompts.py @@ -5,6 +5,116 @@ """ +def prompt_for_chunking_preference(app, nodes, operation_callback): + """ + Prompt user about chunking preference for plotting/histogram operations. + + This function checks if any of the provided nodes are chunked, and + prompts the user to decide whether to use chunked processing. + + Args: + app (H5Forest): + The main application instance. + nodes (list): + List of Node objects to check for chunking. + operation_callback (callable): + Function to call with signature: operation_callback(use_chunks) + where use_chunks is True to use chunked processing. + """ + # Check if config has always_chunk enabled + if app.config.always_chunk_datasets(): + operation_callback(use_chunks=True) + return + + # Check if any node is chunked + has_chunked = any(node.is_chunked for node in nodes) + + # If no chunked data, proceed without chunking + if not has_chunked: + operation_callback(use_chunks=False) + return + + # At this point, we have chunked data and need to prompt + # Calculate memory footprint for all nodes + import h5py + import numpy as np + + total_size_gb = 0.0 + total_chunks = 0 + + try: + for node in nodes: + if node.is_chunked: + with h5py.File(node.filepath, "r") as hdf: + dataset = hdf[node.path] + uncompressed_bytes = dataset.size * dataset.dtype.itemsize + total_size_gb += uncompressed_bytes / (10**9) + + if dataset.chunks: + num_chunks = int( + np.prod( + [ + np.ceil(s / c) + for s, c in zip( + dataset.shape, dataset.chunks + ) + ] + ) + ) + total_chunks += num_chunks + except (OSError, FileNotFoundError, KeyError): + # If file access fails, use node's nbytes as estimate + for node in nodes: + if node.is_chunked: + total_size_gb += node.nbytes / (10**9) + total_chunks = "unknown" + + def on_yes_chunk(): + """User wants chunk-by-chunk processing.""" + app.default_focus() + operation_callback(use_chunks=True) + + def on_no_chunk(): + """User wants to load all at once.""" + app.default_focus() + + def on_yes_load_all(): + """User confirms loading all at once.""" + app.default_focus() + operation_callback(use_chunks=False) + + def on_no_load_all(): + """User wants to abort.""" + app.default_focus() + app.print("Operation aborted.") + + # Ask second question + app.prompt_yn( + "Should we load all at once? (If not, we will abort) [y/n]:", + on_yes_load_all, + on_no_load_all, + ) + + # Build the prompt message + if isinstance(total_chunks, int) and total_chunks > 0: + prompt_msg = ( + f"Chunked Dataset found ({total_size_gb:.2f} GB, " + f"{total_chunks} chunks). " + f"Should we process chunk by chunk? [y/n]:" + ) + else: + prompt_msg = ( + f"Chunked Dataset found ({total_size_gb:.2f} GB). " + f"Should we process chunk by chunk? [y/n]:" + ) + + app.prompt_yn( + prompt_msg, + on_yes_chunk, + on_no_chunk, + ) + + def prompt_for_chunked_dataset( app, node, operation_callback, return_to_normal=True ): diff --git a/src/h5forest/h5_forest.py b/src/h5forest/h5_forest.py index da187c0..e568ec7 100644 --- a/src/h5forest/h5_forest.py +++ b/src/h5forest/h5_forest.py @@ -166,6 +166,7 @@ def _init(self, hdf5_filepath, use_default_config=False): self._flag_plotting_mode = False self._flag_hist_mode = False self._flag_search_mode = False + self._flag_in_prompt = False # Track if we're in a prompt_yn dialog # Timer for debouncing search input self.search_timer = None @@ -640,6 +641,7 @@ def return_to_normal_mode(self): self._flag_plotting_mode = False self._flag_hist_mode = False self._flag_search_mode = False + self._flag_in_prompt = False # Track if we're in a prompt_yn dialog self.mode_title.update_title("Normal Mode") def _init_text_areas(self): @@ -1110,6 +1112,9 @@ def prompt_yn(self, prompt, on_yes, on_no): on_no (callable): Callback to execute when user presses 'n'. """ + # Set the prompt flag to True + self._flag_in_prompt = True + # Set the prompt message self.input_buffer_content.text = prompt self.mini_buffer_content.text = "" @@ -1120,6 +1125,9 @@ def prompt_yn(self, prompt, on_yes, on_no): def cleanup(): """Remove temporary keybindings and clear prompt.""" + # Clear the prompt flag first + self._flag_in_prompt = False + for handler in handlers_to_remove: try: self.kb.bindings.remove(handler) @@ -1143,11 +1151,19 @@ def handle_escape(event): cleanup() on_no() - # Add temporary keybindings for y/n/escape - # These bindings should always be active during the prompt - y_handler = self.kb.add("y")(handle_yes) - n_handler = self.kb.add("n")(handle_no) - esc_handler = self.kb.add("escape")(handle_escape) + # Add temporary keybindings for y/n/escape with filter + # These bindings are ONLY active when _flag_in_prompt is True + from prompt_toolkit.filters import Condition + + y_handler = self.kb.add( + "y", filter=Condition(lambda: self._flag_in_prompt) + )(handle_yes) + n_handler = self.kb.add( + "n", filter=Condition(lambda: self._flag_in_prompt) + )(handle_no) + esc_handler = self.kb.add( + "escape", filter=Condition(lambda: self._flag_in_prompt) + )(handle_escape) handlers_to_remove.extend([y_handler, n_handler, esc_handler]) diff --git a/src/h5forest/plotting.py b/src/h5forest/plotting.py index 2f6b91e..70a3522 100644 --- a/src/h5forest/plotting.py +++ b/src/h5forest/plotting.py @@ -96,31 +96,35 @@ def save_callback(): ) @error_handler - def plot_and_show(self, text): + def plot_and_show(self, text, use_chunks=False): """ Plot the data and show the plot. Args: text (str): The text to extract the plot parameters from. + use_chunks (bool): + Whether to use chunked processing. """ # Compute the plot - self._plot(text) + self._plot(text, use_chunks=use_chunks) # Show the plot self.show() @error_handler - def plot_and_save(self, text): + def plot_and_save(self, text, use_chunks=False): """ Plot the data and save the plot. Args: text (str): The text to extract the plot parameters from. + use_chunks (bool): + Whether to use chunked processing. """ # Compute the plot - self._plot(text) + self._plot(text, use_chunks=use_chunks) # Save the plot self.save() @@ -282,7 +286,7 @@ def reset(self): self.plot_params = {} return self.plot_text - def _plot(self, text): + def _plot(self, text, use_chunks=False): """ Compute a scatter plot of the datasets. @@ -319,14 +323,20 @@ def _plot(self, text): self.ax.set_axisbelow(True) def run_in_thread(): - # Now lets plot the data, if we have chunked data we will plot each - # chunk separately - if ( - x_node.chunks == (1,) - and y_node.chunks == (1,) + # Now lets plot the data + # Use chunk preference to determine if we should load in chunks + # Conditions for loading all at once: + # 1. User preference is to not use chunking (use_chunks == False) + # 2. Neither dataset is chunked + # 3. Datasets have incompatible chunk layouts + should_load_all = ( + not use_chunks + or (x_node.chunks == (1,) and y_node.chunks == (1,)) or x_node.chunks != y_node.chunks - ): - # Get the data + ) + + if should_load_all: + # Get the data all at once with h5py.File(x_node.filepath, "r") as hdf: self.x_data = hdf[x_node.path][...] self.y_data = hdf[y_node.path][...] @@ -524,13 +534,15 @@ def run_in_thread(): return self.plot_text @error_handler - def compute_hist(self, text): + def compute_hist(self, text, use_chunks=False): """ Compute the histogram. Args: text (str): The text to extract the plot parameters from. + use_chunks (bool): + Whether to use chunked processing. """ @error_handler @@ -584,22 +596,24 @@ def run_in_thread(): self.widths = bins[1:] - bins[:-1] self.xs = (bins[1:] + bins[:-1]) / 2 - # Get the number of chunks - chunks = node.chunks if node.is_chunked else 1 + # Use chunk preference to determine if we should load in chunks + # Load all at once if: + # 1. User preference is to not use chunking (use_chunks == False) + # 2. Dataset is not chunked + should_load_all = not use_chunks or not node.is_chunked - # If neither node is not chunked we can just read and grid the data - if chunks == 1: - # Get the data + if should_load_all: + # Get the data all at once with h5py.File(node.filepath, "r") as hdf: data = hdf[node.path][...] - # Compute the grid + # Compute the histogram self.hist, _ = np.histogram(data, bins=bins) - # Otherwise we need to read in the data chunk by chunk and add each - # chunks grid to the total grid else: - # Initialise the grid + # Load in chunks - read the data chunk by chunk and add each + # chunk's histogram to the total + # Initialise the histogram self.hist = np.zeros(nbins) # Get the data @@ -625,7 +639,7 @@ def run_in_thread(): # Get the chunk chunk_data = data[slices] - # Compute the grid for the chunk + # Compute the histogram for the chunk chunk_density, _ = np.histogram( chunk_data, bins=bins ) diff --git a/tests/unit/test_dataset_prompts.py b/tests/unit/test_dataset_prompts.py index 3abb522..1ffb787 100644 --- a/tests/unit/test_dataset_prompts.py +++ b/tests/unit/test_dataset_prompts.py @@ -6,6 +6,7 @@ from h5forest.dataset_prompts import ( prompt_for_chunked_dataset, + prompt_for_chunking_preference, prompt_for_dataset_operation, prompt_for_large_dataset, ) @@ -441,3 +442,309 @@ def test_prompt_for_dataset_operation_with_always_chunk( # Should not show any prompts mock_app.prompt_yn.assert_not_called() + + def test_prompt_for_chunking_preference_always_chunk( + self, mock_app, mock_node + ): + """Test chunking preference with always_chunk config enabled.""" + # Configure mock to enable always_chunk + mock_app.config.always_chunk_datasets.return_value = True + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of nodes + prompt_for_chunking_preference(mock_app, [mock_node], callback) + + # Should call the callback immediately with use_chunks=True + callback.assert_called_once_with(use_chunks=True) + + # Should not show any prompts + mock_app.prompt_yn.assert_not_called() + + def test_prompt_for_chunking_preference_no_chunked_nodes( + self, mock_app, mock_node + ): + """Test chunking preference with no chunked nodes.""" + # Ensure node is not chunked + mock_node.is_chunked = False + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of non-chunked nodes + prompt_for_chunking_preference(mock_app, [mock_node], callback) + + # Should call the callback immediately with use_chunks=False + callback.assert_called_once_with(use_chunks=False) + + # Should not show any prompts + mock_app.prompt_yn.assert_not_called() + + @patch("h5py.File") + def test_prompt_for_chunking_preference_with_chunked_nodes( + self, mock_h5py_file, mock_app, mock_chunked_node + ): + """Test chunking preference with chunked nodes.""" + # Mock the h5py dataset + mock_dataset = Mock() + mock_dataset.size = 1000000 + mock_dataset.dtype.itemsize = 8 + mock_dataset.shape = (1000, 1000) + mock_dataset.chunks = (100, 100) + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock(return_value=mock_dataset) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of chunked nodes + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Should prompt user + mock_app.prompt_yn.assert_called_once() + prompt_msg = mock_app.prompt_yn.call_args[0][0] + # The message should contain chunk count + assert "100 chunks" in prompt_msg + + @patch("h5py.File") + def test_prompt_for_chunking_preference_user_says_yes( + self, mock_h5py_file, mock_app, mock_chunked_node + ): + """Test chunking preference when user chooses to use chunks.""" + # Mock the h5py dataset + mock_dataset = Mock() + mock_dataset.size = 1000000 + mock_dataset.dtype.itemsize = 8 + mock_dataset.shape = (1000, 1000) + mock_dataset.chunks = (100, 100) + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock(return_value=mock_dataset) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of chunked nodes + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Get the yes callback + on_yes = mock_app.prompt_yn.call_args[0][1] + + # Simulate user pressing 'y' + on_yes() + + # Should call callback with use_chunks=True + callback.assert_called_once_with(use_chunks=True) + mock_app.default_focus.assert_called() + + @patch("h5py.File") + def test_prompt_for_chunking_preference_user_says_no( + self, mock_h5py_file, mock_app, mock_chunked_node + ): + """Test chunking preference when user chooses to load all.""" + # Mock the h5py dataset + mock_dataset = Mock() + mock_dataset.size = 1000000 + mock_dataset.dtype.itemsize = 8 + mock_dataset.shape = (1000, 1000) + mock_dataset.chunks = (100, 100) + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock(return_value=mock_dataset) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of chunked nodes + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Get the no callback from first prompt + on_no = mock_app.prompt_yn.call_args[0][2] + + # Simulate user pressing 'n' (triggers second prompt) + on_no() + + # Should have prompted again + assert mock_app.prompt_yn.call_count == 2 + + # Get the yes callback from second prompt + second_on_yes = mock_app.prompt_yn.call_args[0][1] + + # Simulate user pressing 'y' on second prompt + second_on_yes() + + # Should call callback with use_chunks=False + callback.assert_called_once_with(use_chunks=False) + mock_app.default_focus.assert_called() + + @patch("h5py.File") + def test_prompt_for_chunking_preference_multiple_nodes( + self, mock_h5py_file, mock_app + ): + """Test chunking preference with multiple chunked nodes.""" + # Create two chunked nodes + node1 = MagicMock() + node1.is_chunked = True + node1.filepath = "/test/file.h5" + node1.path = "/dataset1" + node1.nbytes = 100 * 10**6 + + node2 = MagicMock() + node2.is_chunked = True + node2.filepath = "/test/file.h5" + node2.path = "/dataset2" + node2.nbytes = 200 * 10**6 + + # Mock the h5py datasets + mock_dataset1 = Mock() + mock_dataset1.size = 1000000 + mock_dataset1.dtype.itemsize = 8 + mock_dataset1.shape = (1000, 1000) + mock_dataset1.chunks = (100, 100) + + mock_dataset2 = Mock() + mock_dataset2.size = 2000000 + mock_dataset2.dtype.itemsize = 8 + mock_dataset2.shape = (2000, 1000) + mock_dataset2.chunks = (200, 100) + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock( + side_effect=lambda key: { + "/dataset1": mock_dataset1, + "/dataset2": mock_dataset2, + }[key] + ) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with multiple nodes + prompt_for_chunking_preference(mock_app, [node1, node2], callback) + + # Should prompt user with combined information + mock_app.prompt_yn.assert_called_once() + prompt_msg = mock_app.prompt_yn.call_args[0][0] + # Message should contain total footprint (0.024 GB ≈ 0.02 GB) + # and total chunk count (100 + 100 = 200) + assert "0.02 GB" in prompt_msg + assert "200 chunks" in prompt_msg + + @patch("h5py.File") + def test_prompt_for_chunking_preference_user_aborts( + self, mock_h5py_file, mock_app, mock_chunked_node + ): + """Test chunking preference when user aborts operation.""" + # Mock the h5py dataset + mock_dataset = Mock() + mock_dataset.size = 1000000 + mock_dataset.dtype.itemsize = 8 + mock_dataset.shape = (1000, 1000) + mock_dataset.chunks = (100, 100) + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock(return_value=mock_dataset) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a list of chunked nodes + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Get the no callback from first prompt + on_no = mock_app.prompt_yn.call_args[0][2] + + # Simulate user pressing 'n' (triggers second prompt) + on_no() + + # Should have prompted again + assert mock_app.prompt_yn.call_count == 2 + + # Get the no callback from second prompt + second_on_no = mock_app.prompt_yn.call_args[0][2] + + # Simulate user pressing 'n' on second prompt (abort) + second_on_no() + + # Should NOT call the operation callback + callback.assert_not_called() + # Should print abort message + mock_app.print.assert_called_with("Operation aborted.") + mock_app.default_focus.assert_called() + + def test_prompt_for_chunking_preference_file_access_error( + self, mock_app, mock_chunked_node + ): + """Test chunking preference when file access fails.""" + # Don't patch h5py.File, let it fail naturally + # This will trigger the exception handler + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a chunked node (will fail to open file) + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Should still prompt user (using nbytes estimate) + mock_app.prompt_yn.assert_called_once() + prompt_msg = mock_app.prompt_yn.call_args[0][0] + # Should contain size estimate from nbytes + assert "0.50 GB" in prompt_msg + # Should not contain chunk count (unknown) + assert "chunks)" not in prompt_msg + + @patch("h5py.File") + def test_prompt_for_chunking_preference_chunks_none( + self, mock_h5py_file, mock_app, mock_chunked_node + ): + """Test chunking preference when dataset.chunks is None.""" + # Mock the h5py dataset with chunks=None + mock_dataset = Mock() + mock_dataset.size = 1000000 + mock_dataset.dtype.itemsize = 8 + mock_dataset.shape = (1000, 1000) + mock_dataset.chunks = None # No chunks defined + + # Mock h5py.File context manager + mock_file = Mock() + mock_file.__enter__ = Mock(return_value=mock_file) + mock_file.__exit__ = Mock(return_value=False) + mock_file.__getitem__ = Mock(return_value=mock_dataset) + mock_h5py_file.return_value = mock_file + + # Create a callback to track if it was called + callback = MagicMock() + + # Call with a chunked node + prompt_for_chunking_preference(mock_app, [mock_chunked_node], callback) + + # Should prompt user without chunk count + mock_app.prompt_yn.assert_called_once() + prompt_msg = mock_app.prompt_yn.call_args[0][0] + # Should contain size but no chunk count (total_chunks is 0) + assert "0.01 GB" in prompt_msg + # Should not contain "chunks)" since total_chunks is 0 + # and fails the > 0 check + assert "chunks)" not in prompt_msg diff --git a/tests/unit/test_hist_bindings.py b/tests/unit/test_hist_bindings.py index db6061e..3c15585 100644 --- a/tests/unit/test_hist_bindings.py +++ b/tests/unit/test_hist_bindings.py @@ -142,13 +142,13 @@ def test_edit_hist_entry_callback(self, mock_app, mock_event): assert "New Title" in mock_app.hist_content.text mock_app.shift_focus.assert_called_with(mock_app.hist_content) - @patch("h5forest.bindings.hist_bindings.prompt_for_dataset_operation") + @patch("h5forest.bindings.hist_bindings.prompt_for_chunking_preference") def test_plot_hist_with_empty_params( self, mock_prompt, mock_app, mock_event ): """Test plotting histogram when params are empty.""" # Make the prompt call the callback immediately - mock_prompt.side_effect = lambda app, node, callback: callback( + mock_prompt.side_effect = lambda app, nodes, callback: callback( use_chunks=False ) @@ -157,6 +157,16 @@ def test_plot_hist_with_empty_params( node = MagicMock() node.is_group = False mock_app.tree.get_current_node = MagicMock(return_value=node) + + # Make set_data_key actually update plot_params + def set_data_key_side_effect(n): + mock_app.histogram_plotter.plot_params["data"] = n + return "data key text" + + mock_app.histogram_plotter.set_data_key.side_effect = ( + set_data_key_side_effect + ) + bindings = [ b for b in mock_app.kb.bindings @@ -168,10 +178,25 @@ def test_plot_hist_with_empty_params( mock_app.histogram_plotter.compute_hist.assert_called_once() mock_app.histogram_plotter.plot_and_show.assert_called_once() - def test_plot_hist_with_existing_params(self, mock_app, mock_event): + @patch("h5forest.bindings.hist_bindings.prompt_for_chunking_preference") + @patch("h5forest.bindings.hist_bindings.WaitIndicator") + def test_plot_hist_with_existing_params( + self, mock_wait_indicator, mock_prompt, mock_app, mock_event + ): """Test plotting histogram with existing params.""" + # Mock WaitIndicator context manager + mock_wait_indicator.return_value.__enter__ = MagicMock() + mock_wait_indicator.return_value.__exit__ = MagicMock() + + # Make the prompt call the callback immediately + mock_prompt.side_effect = lambda app, nodes, callback: callback( + use_chunks=False + ) + _init_hist_bindings(mock_app) - mock_app.histogram_plotter.plot_params = {"data": "test"} + # Create a mock node for plot_params + mock_node = MagicMock() + mock_app.histogram_plotter.plot_params = {"data": mock_node} bindings = [ b for b in mock_app.kb.bindings @@ -199,10 +224,25 @@ def test_plot_hist_with_group_node(self, mock_app, mock_event): handler(mock_event) mock_app.print.assert_called_once_with("/group is not a Dataset") - def test_save_hist(self, mock_app, mock_event): + @patch("h5forest.bindings.hist_bindings.prompt_for_chunking_preference") + @patch("h5forest.bindings.hist_bindings.WaitIndicator") + def test_save_hist( + self, mock_wait_indicator, mock_prompt, mock_app, mock_event + ): """Test saving histogram.""" + # Mock WaitIndicator context manager + mock_wait_indicator.return_value.__enter__ = MagicMock() + mock_wait_indicator.return_value.__exit__ = MagicMock() + + # Make the prompt call the callback immediately + mock_prompt.side_effect = lambda app, nodes, callback: callback( + use_chunks=False + ) + _init_hist_bindings(mock_app) - mock_app.histogram_plotter.plot_params = {"data": "test"} + # Create a mock node for plot_params + mock_node = MagicMock() + mock_app.histogram_plotter.plot_params = {"data": mock_node} bindings = [ b for b in mock_app.kb.bindings @@ -229,13 +269,13 @@ def test_save_hist_with_group_node(self, mock_app, mock_event): handler(mock_event) mock_app.print.assert_called_once_with("/group is not a Dataset") - @patch("h5forest.bindings.hist_bindings.prompt_for_dataset_operation") + @patch("h5forest.bindings.hist_bindings.prompt_for_chunking_preference") def test_save_hist_with_empty_params( self, mock_prompt, mock_app, mock_event ): """Test saving histogram with empty params and dataset.""" # Make the prompt call the callback immediately - mock_prompt.side_effect = lambda app, node, callback: callback( + mock_prompt.side_effect = lambda app, nodes, callback: callback( use_chunks=False ) @@ -245,8 +285,14 @@ def test_save_hist_with_empty_params( node.is_group = False node.path = "/dataset" mock_app.tree.get_current_node = MagicMock(return_value=node) + + # Make set_data_key actually update plot_params + def set_data_key_side_effect(n): + mock_app.histogram_plotter.plot_params["data"] = n + return "data key text" + mock_app.histogram_plotter.set_data_key = MagicMock( - return_value="data key text" + side_effect=set_data_key_side_effect ) mock_app.histogram_plotter.compute_hist = MagicMock( return_value="computed hist" @@ -315,13 +361,8 @@ def test_exit_edit_hist(self, mock_app, mock_event): handler(mock_event) mock_app.shift_focus.assert_called_with(mock_app.tree_content) - @patch("h5forest.bindings.hist_bindings.prompt_for_dataset_operation") - def test_select_data(self, mock_prompt, mock_app, mock_event): + def test_select_data(self, mock_app, mock_event): """Test selecting data for histogram.""" - # Make the prompt call the callback immediately - mock_prompt.side_effect = lambda app, node, callback: callback( - use_chunks=False - ) _init_hist_bindings(mock_app) node = MagicMock() @@ -653,3 +694,51 @@ def test_all_keys_bound(self, mock_app): for key in ["c-m", "b", "x", "y", "h", "H", "r", "e", "q"]: bindings = [b for b in mock_app.kb.bindings if key in str(b.keys)] assert len(bindings) > 0, f"Key '{key}' not bound" + + def test_plot_hist_missing_data(self, mock_app, mock_event): + """Test plotting histogram without data selected.""" + _init_hist_bindings(mock_app) + + # Set plot_params with other keys but not "data" + # This skips the "if len(plot_params) == 0" block + # but triggers the "if 'data' not in plot_params" check + mock_app.histogram_plotter.plot_params = {"other_key": "value"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("h",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + mock_app.print.assert_called_once_with( + "Please select a dataset first (Enter)" + ) + # Should not call compute_hist + mock_app.histogram_plotter.compute_hist.assert_not_called() + + def test_save_hist_missing_data(self, mock_app, mock_event): + """Test saving histogram without data selected.""" + _init_hist_bindings(mock_app) + + # Set plot_params with other keys but not "data" + # This skips the "if len(plot_params) == 0" block + # but triggers the "if 'data' not in plot_params" check + mock_app.histogram_plotter.plot_params = {"other_key": "value"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("H",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + mock_app.print.assert_called_once_with( + "Please select a dataset first (Enter)" + ) + # Should not call compute_hist + mock_app.histogram_plotter.compute_hist.assert_not_called() diff --git a/tests/unit/test_plot_bindings.py b/tests/unit/test_plot_bindings.py index 89fa836..e89c03c 100644 --- a/tests/unit/test_plot_bindings.py +++ b/tests/unit/test_plot_bindings.py @@ -278,8 +278,14 @@ def test_edit_plot_entry_callback(self, mock_app, mock_event): assert "New Title" in mock_app.plot_content.text mock_app.shift_focus.assert_called_once_with(mock_app.plot_content) - def test_plot_scatter(self, mock_app, mock_event): + @patch("h5forest.bindings.plot_bindings.prompt_for_chunking_preference") + def test_plot_scatter(self, mock_prompt, mock_app, mock_event): """Test plotting and showing scatter plot.""" + # Make the prompt call the callback immediately + mock_prompt.side_effect = lambda app, nodes, callback: callback( + use_chunks=False + ) + _init_plot_bindings(mock_app) bindings = [ @@ -292,13 +298,19 @@ def test_plot_scatter(self, mock_app, mock_event): handler = bindings[0].handler handler(mock_event) - # Verify plot_and_show was called + # Verify plot_and_show was called with use_chunks parameter mock_app.scatter_plotter.plot_and_show.assert_called_once_with( - mock_app.plot_content.text + mock_app.plot_content.text, use_chunks=False ) - def test_save_scatter(self, mock_app, mock_event): + @patch("h5forest.bindings.plot_bindings.prompt_for_chunking_preference") + def test_save_scatter(self, mock_prompt, mock_app, mock_event): """Test saving scatter plot.""" + # Make the prompt call the callback immediately + mock_prompt.side_effect = lambda app, nodes, callback: callback( + use_chunks=False + ) + _init_plot_bindings(mock_app) bindings = [ @@ -311,10 +323,153 @@ def test_save_scatter(self, mock_app, mock_event): handler = bindings[0].handler handler(mock_event) - # Verify plot_and_save was called + # Verify plot_and_save was called with use_chunks parameter mock_app.scatter_plotter.plot_and_save.assert_called_once_with( - mock_app.plot_content.text + mock_app.plot_content.text, use_chunks=False + ) + + def test_plot_scatter_missing_x_axis(self, mock_app, mock_event): + """Test plotting without x-axis selected.""" + _init_plot_bindings(mock_app) + + # Set plot_params with only y + mock_app.scatter_plotter.plot_params = {"y": "some_data"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("p",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + expected_msg = ( + "Please select both x-axis (x) and y-axis (y) datasets first" + ) + mock_app.print.assert_called_once_with(expected_msg) + # Should not call plot_and_show + mock_app.scatter_plotter.plot_and_show.assert_not_called() + + def test_plot_scatter_missing_y_axis(self, mock_app, mock_event): + """Test plotting without y-axis selected.""" + _init_plot_bindings(mock_app) + + # Set plot_params with only x + mock_app.scatter_plotter.plot_params = {"x": "some_data"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("p",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + expected_msg = ( + "Please select both x-axis (x) and y-axis (y) datasets first" + ) + mock_app.print.assert_called_once_with(expected_msg) + # Should not call plot_and_show + mock_app.scatter_plotter.plot_and_show.assert_not_called() + + def test_plot_scatter_missing_both_axes(self, mock_app, mock_event): + """Test plotting without any axes selected.""" + _init_plot_bindings(mock_app) + + # Set plot_params empty + mock_app.scatter_plotter.plot_params = {} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("p",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + expected_msg = ( + "Please select both x-axis (x) and y-axis (y) datasets first" ) + mock_app.print.assert_called_once_with(expected_msg) + # Should not call plot_and_show + mock_app.scatter_plotter.plot_and_show.assert_not_called() + + def test_save_scatter_missing_x_axis(self, mock_app, mock_event): + """Test saving without x-axis selected.""" + _init_plot_bindings(mock_app) + + # Set plot_params with only y + mock_app.scatter_plotter.plot_params = {"y": "some_data"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("P",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + expected_msg = ( + "Please select both x-axis (x) and y-axis (y) datasets first" + ) + mock_app.print.assert_called_once_with(expected_msg) + # Should not call plot_and_save + mock_app.scatter_plotter.plot_and_save.assert_not_called() + + def test_save_scatter_missing_y_axis(self, mock_app, mock_event): + """Test saving without y-axis selected.""" + _init_plot_bindings(mock_app) + + # Set plot_params with only x + mock_app.scatter_plotter.plot_params = {"x": "some_data"} + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("P",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Should print error message + expected_msg = ( + "Please select both x-axis (x) and y-axis (y) datasets first" + ) + mock_app.print.assert_called_once_with(expected_msg) + # Should not call plot_and_save + mock_app.scatter_plotter.plot_and_save.assert_not_called() + + @patch("h5forest.bindings.plot_bindings.WaitIndicator") + @patch("h5forest.bindings.plot_bindings.prompt_for_chunking_preference") + def test_plot_scatter_calls_default_focus( + self, mock_prompt, mock_wait_indicator, mock_app, mock_event + ): + """Test that plot_scatter calls default_focus after plotting.""" + # Mock WaitIndicator context manager + mock_wait_indicator.return_value.__enter__ = MagicMock() + mock_wait_indicator.return_value.__exit__ = MagicMock() + + # Make the prompt call the callback immediately + mock_prompt.side_effect = lambda app, nodes, callback: callback( + use_chunks=False + ) + + _init_plot_bindings(mock_app) + + bindings = [ + b + for b in mock_app.kb.bindings + if b.keys == ("p",) and b.filter is not None + ] + handler = bindings[0].handler + handler(mock_event) + + # Verify default_focus was called + mock_app.default_focus.assert_called_once() def test_reset(self, mock_app, mock_event): """Test resetting plot content.""" diff --git a/tests/unit/test_plotting.py b/tests/unit/test_plotting.py index d5a2322..90112f6 100644 --- a/tests/unit/test_plotting.py +++ b/tests/unit/test_plotting.py @@ -128,9 +128,9 @@ def test_plot_and_show(self, mock_show): plotter._plot = Mock() text = "test plot text" - plotter.plot_and_show(text) + plotter.plot_and_show(text, use_chunks=False) - plotter._plot.assert_called_once_with(text) + plotter._plot.assert_called_once_with(text, use_chunks=False) mock_show.assert_called_once() @patch("h5forest.plotting.Path.cwd") @@ -148,9 +148,9 @@ def test_plot_and_save(self, mock_forest_class, mock_cwd): plotter.fig.savefig = Mock() text = "test plot text" - plotter.plot_and_save(text) + plotter.plot_and_save(text, use_chunks=False) - plotter._plot.assert_called_once_with(text) + plotter._plot.assert_called_once_with(text, use_chunks=False) mock_forest.input.assert_called_once() @@ -530,10 +530,10 @@ def y_getitem(key): "marker: .\n" ) - plotter._plot(text) + plotter._plot(text, use_chunks=True) - # Verify scatter was called multiple times - # (np.ndindex with chunks=(5,) gives 5 iterations) + # Verify scatter was called 5 times (once per chunk) + # np.ndindex((5,)) gives (0,), (1,), (2,), (3,), (4,) = 5 iterations assert mock_ax.scatter.call_count == 5 @patch("h5forest.plotting.plt.figure") @@ -1744,14 +1744,16 @@ def test_full_scatter_plot_workflow( @patch("h5forest.progress.get_window_size") @patch("h5forest.plotting.plt.show") - @patch("h5forest.plotting.get_app") + @patch("h5forest.h5_forest.H5Forest") def test_full_histogram_workflow( - self, mock_get_app, mock_show, mock_window_size, histogram_h5_file + self, mock_forest_class, mock_show, mock_window_size, histogram_h5_file ): """Test complete histogram workflow with real HDF5 file.""" # Setup mocks mock_app = Mock() - mock_get_app.return_value = mock_app + mock_app.mini_buffer_content = Mock() + mock_app.mini_buffer_content.text = "" + mock_forest_class.return_value = mock_app mock_window_size.return_value = (24, 80) # Create plotter @@ -1784,14 +1786,14 @@ def test_full_histogram_workflow( node.is_chunked = False # Compute histogram - plotter.compute_hist(text) + plotter.compute_hist(text, use_chunks=False) # Wait for thread if plotter.compute_hist_thread: plotter.compute_hist_thread.join() # Create plot - plotter.plot_and_show(text) + plotter.plot_and_show(text, use_chunks=False) # Verify plot was shown mock_show.assert_called_once()