Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0f3293f
move examples under pdf dir
cadenmyers13 Sep 23, 2025
c5c9f84
edit cli to reflect new path change
cadenmyers13 Sep 23, 2025
1092dcb
news
cadenmyers13 Sep 23, 2025
14102ad
fix typo
cadenmyers13 Sep 23, 2025
d423377
Revise examples in news/ex-path.rst for clarity
sbillinge Sep 24, 2025
683b4c3
rm type hint
cadenmyers13 Sep 25, 2025
71f9de4
merge to local: move news from added to changed
cadenmyers13 Sep 25, 2025
3a1ee8b
change list_examples to map_pack_to_examples which returns a dict
cadenmyers13 Sep 25, 2025
d3b531c
change variable name to reflect action properly
cadenmyers13 Sep 25, 2025
73d083e
change function name
cadenmyers13 Sep 25, 2025
61a9bd6
update docstring
cadenmyers13 Sep 25, 2025
012a45e
Merge branch 'main' of github.com:diffpy/diffpy.cmi into ex-path
cadenmyers13 Sep 25, 2025
d90a271
tests for help command, listing examples, and copying examples
cadenmyers13 Sep 25, 2025
c12f6b5
fix monkeypatch debugging code
cadenmyers13 Sep 25, 2025
c0bead8
update function names to be more readable
cadenmyers13 Sep 26, 2025
63ac630
test map pack to examples function
cadenmyers13 Sep 26, 2025
e0a2c75
add no packs, no examples case
cadenmyers13 Sep 26, 2025
81fa5cf
make sure directory was created
cadenmyers13 Sep 26, 2025
10771b1
add tests for copy_example
cadenmyers13 Sep 26, 2025
4ee36af
Add Error if pack or example doesnt exist
cadenmyers13 Sep 26, 2025
4019e36
Merge branch 'main' ex-path to see core example
cadenmyers13 Sep 26, 2025
65fd2c7
map packs function test, simplified
cadenmyers13 Sep 29, 2025
74e992b
add condition that catches extra slashes
cadenmyers13 Sep 29, 2025
d53fab8
test copy behavior
cadenmyers13 Sep 29, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/ex-path.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* Add examples under specific pack name.

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
65 changes: 51 additions & 14 deletions src/diffpy/cmi/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,40 @@ def _installed_examples_dir() -> Path:


def list_examples() -> List[str]:
"""List installed example names.
"""List installed examples grouped by pack.

Returns
-------
list of str
Installed example directory names.
dict[str, list[str]]
Mapping of pack name -> list of example subdirectories.
"""
root = _installed_examples_dir()
if not root.exists():
return []
return sorted([p.name for p in root.iterdir() if p.is_dir()])
return {}

packs: dict[str, list[str]] = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you are using chatgpt pleaes review the code very carefully. We are not using type-hints atm so remove all type hints. Also, this is a lot of code now. Is it all necessary or there is a more elegant way of accomplishing the same thing?

My life gets way harder when I have to review crappy code written by chatgpt. At least when I spend time reviewing crappy code written by students I feel that the student is learning and in the future, as a result, I will have to spend less time reviewing.....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there tests for this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbillinge sorry about that... I do review any gpt generated code and try my best to use it as a tool and not to be lazy. I'll scrutinize it more in the future.

Here's my idea for how users can see and copy the examples. I want the examples to be listed under each pack like you see in the screenshot above where its something like this:

$ cmi example list
pack1:
 - exampleA
 - exampleB
pack2:
 - exampleC
 - exampleD

This code accomplishes this by creating a dict and storing the pack dir names as keys and a list of the examples as the value. Then, I want users to be able to copy the example to there cwd via cmi copy example pack1/exampleA. The upstream configuration copies examples by running cmi example copy exampleA, as more packs get added to cmi you could imagine how this would become a mess. You may have interpreted this from this PR already but I just want to be explicit in my objectives. Can you explain a bit more why you think this code is crappy?

Tieqiong wrote a CI test for this here that runs the commands but i don't see any tests for listing examples (cmi example list) or copying them (cmi example copy <ex name>)

for pack_dir in sorted(root.iterdir()):
if not pack_dir.is_dir():
continue
pack = pack_dir.name
exdirs = sorted([p.name for p in pack_dir.iterdir() if p.is_dir()])
packs[pack] = exdirs
return packs


def print_examples() -> None:
"""Print the installed examples grouped by pack."""
packs = list_examples()
if not packs:
print("No examples found.")
return
for pack, exdirs in packs.items():
print(pack + ":")
if not exdirs:
print(" (no examples)")
else:
for ex in exdirs:
print(f" - {ex}")


def copy_example(example: str) -> Path:
Expand All @@ -72,7 +95,7 @@ def copy_example(example: str) -> Path:
Parameters
----------
example : str
Example directory name under the installed examples root.
Example in the form ``<pack>/<exdir>``.

Returns
-------
Expand All @@ -81,17 +104,25 @@ def copy_example(example: str) -> Path:

Raises
------
ValueError
If the format is invalid.
FileNotFoundError
If the example directory does not exist.
FileExistsError
If the destination directory already exists.
"""
src = _installed_examples_dir() / example
if "/" not in example:
raise ValueError("Example must be specified as <pack>/<exdir>")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enforces new format of copying examples. This format is cmi example copy <pack>/<exdir>


pack, exdir = example.split("/", 1)
src = _installed_examples_dir() / pack / exdir
if not src.exists() or not src.is_dir():
raise FileNotFoundError(f"Example not found: {example}")
dest = Path.cwd() / example

dest = Path.cwd() / exdir
if dest.exists():
raise FileExistsError(f"Destination {dest} already exists")

copytree(src, dest)
return dest

Expand Down Expand Up @@ -163,7 +194,9 @@ def _build_parser() -> argparse.ArgumentParser:
_parser=p_example
)
p_example_copy = sub_ex.add_parser("copy", help="Copy an example to CWD")
p_example_copy.add_argument("name", metavar="EXAMPLE", help="Example name")
p_example_copy.add_argument(
"name", metavar="EXAMPLE", help="Example name <pack>/<exdir>"
)
p_example_copy.set_defaults(_parser=p_example)
p_example.set_defaults(example_cmd=None)

Expand Down Expand Up @@ -335,12 +368,16 @@ def _cmd_example(ns: argparse.Namespace) -> int:
)
ns._parser.print_help()
return 1
out = copy_example(name)
print(f"Example copied to: {out}")
return 0
try:
out = copy_example(name)
print(f"Example copied to: {out}")
return 0
except (ValueError, FileNotFoundError, FileExistsError) as e:
plog.error("%s", e)
return 1

if ns.example_cmd == "list":
for g in list_examples():
print(g)
print_examples()
return 0

plog.error("Unknown example subcommand.")
Expand Down
Loading