Windows compatibility by xiaoyuleiba · Pull Request #39 · has2k1/gnuplot_kernel

xiaoyuleiba · 2026-03-22T07:01:02Z

The summaries below are auto-generated by copilot

This pull request introduces several improvements and fixes to the gnuplot kernel's magic registration, command parsing, and statement recognition. The changes enhance the clarity and reliability of magic command registration, improve newline handling in the REPL wrapper, and refine regular expressions for plot statement detection.

Magic registration improvements:

Explicitly names the magic functions for both line and cell magics as gnuplot, improving clarity and preventing accidental registration of unnamed magics. (src/gnuplot_kernel/magics/gnuplot_magic.py)

REPL wrapper and command handling:

Uses a defined CRLF constant for sending commands to the gnuplot process, ensuring consistent newline handling across platforms. (src/gnuplot_kernel/replwrap.py)
Adjusts the prompt removal logic to use strip() instead of strip(" "), ensuring all leading and trailing whitespace is removed from command outputs. (src/gnuplot_kernel/replwrap.py)

Regular expression and statement parsing:

Refines the PLOT_RE regular expression to use a non-capturing group for plot command variants, adds a word boundary and whitespace/end-of-line checks, and updates the named group from plot_cmd to cmd for clarity and correctness. (src/gnuplot_kernel/statement.py)

…lotKernel

Copilot

Pull request overview

This PR aims to improve Windows compatibility and general robustness of the gnuplot Jupyter kernel by tightening statement detection, standardizing newline handling to CRLF, making magic registration explicit, and adding Windows-oriented safeguards around inline plot file generation/reading/deletion.

Changes:

Refines plot-statement detection regex to reduce false positives and improve token boundary handling.
Standardizes REPL command newline handling via a CRLF constant and improves prompt/output whitespace stripping.
Adds Windows-focused inline image handling in the kernel (tempdir paths, read/write quiescence, retry reads, deferred deletion) and makes IPython magic registration explicit.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File	Description
`src/gnuplot_kernel/statement.py`	Updates plot statement regex; leaves commented legacy regex in place.
`src/gnuplot_kernel/replwrap.py`	Sends commands using `CRLF` and strips output with `strip()`.
`src/gnuplot_kernel/magics/gnuplot_magic.py`	Explicitly names the registered line/cell magics as `"gnuplot"`.
`src/gnuplot_kernel/kernel.py`	Adds Windows-specific logic for inline image file creation/reading/deletion and prompt handling; introduces new helper methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T09:36:14Z

src/gnuplot_kernel/kernel.py

+    _pending_unlink: list[Path] = []
+


_pending_unlink is declared as a class attribute but then mutated (append/assign) via self._pending_unlink. This can leak state across kernel instances and across tests; make it an instance attribute (e.g., initialize in __init__ or on first use) to avoid cross-instance interference.

Suggested change

_pending_unlink: list[Path] = []

# Class-level sentinel; actual list is per-instance (see __init__).

_pending_unlink: list[Path] | None = None

def __init__(self, *args, **kwargs) -> None:

super().__init__(*args, **kwargs)

# Each kernel instance gets its own list of paths to unlink.

self._pending_unlink: list[Path] = []

Copilot · 2026-03-22T09:36:14Z

src/gnuplot_kernel/kernel.py

+            for p in self._pending_unlink:
+                try:
+                    p.unlink()
+                except (FileNotFoundError, PermissionError):


The deferred-unlink cleanup keeps paths even when unlink() raises FileNotFoundError. That will cause already-deleted files to stay in _pending_unlink forever and be retried every execution. Only keep entries on PermissionError (and drop them on FileNotFoundError).

Suggested change

except (FileNotFoundError, PermissionError):

except FileNotFoundError:

# File already removed; do not keep it in the pending list.

pass

except PermissionError:

# Still cannot remove; keep for a later retry.

Copilot · 2026-03-22T09:36:14Z

src/gnuplot_kernel/kernel.py

+                except (FileNotFoundError, PermissionError):
+                    self._pending_unlink.append(filename)
+
+


In delete_image_files, FileNotFoundError is treated the same as PermissionError and the filename is added to _pending_unlink. If the file is already gone, it shouldn’t be queued for retry; only enqueue on PermissionError and ignore missing files.

Suggested change

except (FileNotFoundError, PermissionError):

self._pending_unlink.append(filename)

except PermissionError:

self._pending_unlink.append(filename)

except FileNotFoundError:

# If the file is already gone, there is nothing to retry.

pass

Copilot · 2026-03-22T09:36:14Z

src/gnuplot_kernel/kernel.py

    def display_images(self):
-        """
-        Display images if gnuplot wrote to them
-        """
+        # Key fix: do not start reading before gnuplot finishes emitting
+        # all images for this cell.
+        if os.name == "nt" and self.inline_plotting:  
+            files0 = self._iter_inline_candidates()
+            max_sz = 0
+            for p in files0:
+                try:
+                    max_sz = max(max_sz, p.stat().st_size)
+                except FileNotFoundError:
+                    pass
+            if max_sz >= 10_000_000:
+                self._wait_inline_quiescence(timeout=8.0, poll=0.02, settle=0.18)
+            else:
+                self._wait_inline_quiescence(timeout=2.0, poll=0.01, settle=0.08)
+


This PR introduces substantial new Windows-specific behavior in display_images (quiescence wait + retry reads + deferred deletion), but there are no tests exercising these code paths. Since this repo already has kernel execution tests, please add targeted tests (e.g., monkeypatch gnuplot_kernel.kernel.os.name to "nt" and validate pending-unlink cleanup / read-retry behavior on temp files) to prevent regressions.

Copilot · 2026-03-22T09:36:14Z

src/gnuplot_kernel/statement.py

+# PLOT_RE = re.compile(
+#     r"^\s*"
+#     r"(?P<plot_cmd>"
+#     r"plot|plo|pl|p|"
+#     r"splot|splo|spl|sp|"
+#     r"replot|replo|repl|rep"
+#     r")"
+#     r"\s?"
+# )


Large commented-out block of the previous PLOT_RE implementation is left in the file. This makes the regex section harder to read/maintain and duplicates information that git history already preserves; please remove the commented-out legacy regex.

Suggested change

# PLOT_RE = re.compile(

# r"^\s*"

# r"(?P<plot_cmd>"

# r"plot|plo|pl|p|"

# r"splot|splo|spl|sp|"

# r"replot|replo|repl|rep"

# r")"

# r"\s?"

# )

Copilot · 2026-03-22T09:36:15Z

src/gnuplot_kernel/replwrap.py


+            # retval = PROMPT_REMOVE_RE.sub("", retval).strip(" ")


There’s leftover commented-out code and extra blank lines around the prompt-stripping logic. Since strip() is now the intended behavior, please remove the commented-out strip(" ") line to keep the method concise and avoid confusion about which behavior is correct.

Suggested change

# retval = PROMPT_REMOVE_RE.sub("", retval).strip(" ")

Copilot · 2026-03-22T09:36:15Z

src/gnuplot_kernel/kernel.py

 from __future__ import annotations
-
+import os
+import time
+from IPython.display import Image, SVG
+import re
+import tempfile
 import contextlib
 import sys
 import uuid
 from itertools import chain
 from pathlib import Path
-from typing import cast
-
+from typing import cast, Iterable 
 from IPython.display import SVG, Image
 from metakernel import MetaKernel, ProcessMetaKernel, pexpect


The import section now has duplicate imports (Image, SVG imported twice) and an unused import (Iterable). This will trip ruff’s F401/I001 rules; please remove duplicates/unused imports and reorder imports into the project’s standard grouping.

Copilot · 2026-03-22T09:36:15Z

src/gnuplot_kernel/kernel.py

                msg = (
-                    "Failed to read and display image file from gnuplot."
-                    "Possibly:\n"
+                    "Failed to read and display image file from gnuplot.Possibly:\n"


Error message string concatenation lost spacing/newline: "... from gnuplot.Possibly:\n" reads as a single sentence. Please restore a space or newline after the period (e.g., gnuplot. Possibly:) so the message is readable.

Suggested change

"Failed to read and display image file from gnuplot.Possibly:\n"

"Failed to read and display image file from gnuplot. Possibly:\n"

Copilot · 2026-03-22T09:36:15Z

src/gnuplot_kernel/kernel.py

+                self._wait_inline_quiescence(timeout=2.0, poll=0.01, settle=0.08)
+
+
+                # 非阻塞清理上一轮未删掉的文件


The comment # 非阻塞清理上一轮未删掉的文件 is indented as if it were inside the preceding if block, but it actually documents the following cleanup if. Please fix the indentation so the comment aligns with the code it describes.

Suggested change

# 非阻塞清理上一轮未删掉的文件

# 非阻塞清理上一轮未删掉的文件

Copilot · 2026-03-22T09:36:15Z

src/gnuplot_kernel/kernel.py

+    #     else:
+    #         return
+
+    #     for filename in self.iter_image_files():
+    #         try:
+    #             size = filename.stat().st_size
+    #         except FileNotFoundError:
+    #             size = 0
+
+    #         if not size:
+    #             msg = (
+    #                 "Failed to read and display image file from gnuplot."
+    #                 "Possibly:\n"
+    #                 "1. You have plotted to a non interactive terminal.\n"
+    #                 "2. You have an invalid expression."
+    #             )
+    #             print(msg)
+    #             continue
+
+    #         im = _Image(str(filename))
+    #         self.Display(im)
+
+
+
+


There are multiple large blocks of commented-out previous implementations (e.g., old get_image_filename / display_images). These make the file significantly harder to maintain and review; please remove the dead commented code and rely on git history for reference.

Suggested change

# else:

# return

# for filename in self.iter_image_files():

# try:

# size = filename.stat().st_size

# except FileNotFoundError:

# size = 0

# if not size:

# msg = (

# "Failed to read and display image file from gnuplot."

# "Possibly:\n"

# "1. You have plotted to a non interactive terminal.\n"

# "2. You have an invalid expression."

# )

# print(msg)

# continue

# im = _Image(str(filename))

# self.Display(im)

xiaoyuleiba added 2 commits February 28, 2026 02:56

enh: Improve Windows compatibility and enhance image handling in Gnup…

a76027c

…lotKernel

enh: Improve Windows file handling and stability in GnuplotKernel

9acdc2f

xiaoyuleiba marked this pull request as ready for review March 22, 2026 09:32

Copilot AI review requested due to automatic review settings March 22, 2026 09:32

Copilot started reviewing on behalf of xiaoyuleiba March 22, 2026 09:32 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows compatibility#39

Windows compatibility#39
xiaoyuleiba wants to merge 2 commits intohas2k1:mainfrom
xiaoyuleiba:windows_compatibility

xiaoyuleiba commented Mar 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    _pending_unlink: list[Path] = []
+    # Class-level sentinel; actual list is per-instance (see __init__).
+    _pending_unlink: list[Path] | None = None
+    def __init__(self, *args, **kwargs) -> None:
+        super().__init__(*args, **kwargs)
+        # Each kernel instance gets its own list of paths to unlink.
+        self._pending_unlink: list[Path] = []

-                except (FileNotFoundError, PermissionError):
+                except FileNotFoundError:
+                    # File already removed; do not keep it in the pending list.
+                    pass
+                except PermissionError:
+                    # Still cannot remove; keep for a later retry.

		except (FileNotFoundError, PermissionError):
		self._pending_unlink.append(filename)

	"Failed to read and display image file from gnuplot.Possibly:\n"
	"Failed to read and display image file from gnuplot. Possibly:\n"

		self._wait_inline_quiescence(timeout=2.0, poll=0.01, settle=0.08)


		# 非阻塞清理上一轮未删掉的文件

Conversation

xiaoyuleiba commented Mar 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants