-
Notifications
You must be signed in to change notification settings - Fork 5
test: copy_examples test
#65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
17e3747
82f7394
d973c83
375606c
711c24e
987c3e3
9e65a27
5354831
47d87fb
0a490b9
6e2695d
e96988f
abfba68
d4cddf5
38881b8
f4fe605
07cbcd7
078b13c
e7444d7
c792c0b
7d7a2a3
52a6752
611bdf5
d6f34e0
087851b
87c1f43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| **Added:** | ||
|
|
||
| * Add test for ``copy_examples``. | ||
|
|
||
| **Changed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Deprecated:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Removed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Fixed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Security:** | ||
|
|
||
| * <news item> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,4 @@ PyYAML | |
| diffpy.utils | ||
| diffpy.srfit | ||
| diffpy.structure | ||
| rich | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,11 +12,13 @@ | |
| # See LICENSE.rst for license information. | ||
| # | ||
| ############################################################################## | ||
|
|
||
| import shutil | ||
| from importlib.resources import as_file | ||
| from pathlib import Path | ||
| from typing import List, Union | ||
|
|
||
| from rich.console import Console | ||
|
|
||
| from diffpy.cmi.installer import ( | ||
| ParsedReq, | ||
| install_requirements, | ||
|
|
@@ -79,6 +81,7 @@ class PacksManager: | |
| def __init__(self, root_path=None) -> None: | ||
| self.packs_dir = _installed_packs_dir(root_path) | ||
| self.examples_dir = self._get_examples_dir() | ||
| self.console = Console() | ||
|
|
||
| def _get_examples_dir(self) -> Path: | ||
| """Return the absolute path to the installed examples directory. | ||
|
|
@@ -133,6 +136,133 @@ def available_examples(self) -> dict[str, List[tuple[str, Path]]]: | |
| ) | ||
| return examples_dict | ||
|
|
||
| def copy_examples( | ||
| self, | ||
| examples_to_copy: List[str], | ||
| target_dir: Path = None, | ||
| force: bool = False, | ||
cadenmyers13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) -> None: | ||
| """Copy examples or packs into the target or current working | ||
| directory. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| examples_to_copy : list of str | ||
| User-specified pack(s), example(s), or "all" to copy all. | ||
| target_dir : pathlib.Path, optional | ||
| Target directory to copy examples into. Defaults to current | ||
| working directory. | ||
| force : bool, optional | ||
| Defaults to ``False``. If ``True``, existing files are | ||
| overwritten and directories are merged | ||
| (extra files in the target are preserved). | ||
| """ | ||
| self._target_dir = target_dir.resolve() if target_dir else Path.cwd() | ||
| self._force = force | ||
|
|
||
| if "all" in examples_to_copy: | ||
| self._copy_all() | ||
| return | ||
|
|
||
| for item in examples_to_copy: | ||
| if item in self.available_examples(): | ||
| self._copy_pack(item) | ||
| elif self._is_example_name(item): | ||
| self._copy_example(item) | ||
| else: | ||
| raise FileNotFoundError( | ||
| f"No examples or packs found for input: '{item}'" | ||
| ) | ||
| del self._target_dir | ||
| del self._force | ||
| return | ||
|
|
||
| def _copy_all(self): | ||
| """Copy all packs and examples.""" | ||
| for pack_name in self.available_examples(): | ||
| self._copy_pack(pack_name) | ||
|
|
||
| def _copy_pack(self, pack_name): | ||
| """Copy all examples in a single pack.""" | ||
| examples = self.available_examples().get(pack_name, []) | ||
| for ex_name, ex_path in examples: | ||
| self._copy_tree_to_target(pack_name, ex_name, ex_path) | ||
|
|
||
| def _copy_example(self, example_name): | ||
cadenmyers13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """Copy a single example by its name.""" | ||
| example_found = False | ||
| for pack_name, examples in self.available_examples().items(): | ||
| for ex_name, ex_path in examples: | ||
| if ex_name == example_name: | ||
| self._copy_tree_to_target(pack_name, ex_name, ex_path) | ||
| example_found = True | ||
| if not example_found: | ||
| raise FileNotFoundError( | ||
| f"No examples or packs found for input: '{example_name}'" | ||
| ) | ||
|
|
||
| def _is_example_name(self, name): | ||
| """Return True if the given name matches any known example.""" | ||
| for pack_name, examples in self.available_examples().items(): | ||
| for example_name, _ in examples: | ||
| if example_name == name: | ||
| return True | ||
| return False | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Main update on recent push is the content below here. I've also found this package called |
||
| def _copy_tree_to_target(self, pack_name, example_name, example_origin): | ||
| """Copy an example folder from source to the user's target | ||
| directory.""" | ||
| target_dir = self._target_dir / pack_name / example_name | ||
| target_dir.parent.mkdir(parents=True, exist_ok=True) | ||
| if target_dir.exists() and self._force: | ||
| self._overwrite_example( | ||
| example_origin, target_dir, pack_name, example_name | ||
| ) | ||
| return | ||
| if target_dir.exists(): | ||
| self._copy_missing_files(example_origin, target_dir) | ||
| print( | ||
| f"WARNING: Example '{pack_name}/{example_name}' " | ||
| "already exists at the specified target directory. " | ||
| "Existing files were left unchanged; " | ||
| "new or missing files were copied. To overwrite everything, " | ||
| "rerun with --force." | ||
| ) | ||
| return | ||
| self._copy_new_example( | ||
| example_origin, target_dir, pack_name, example_name | ||
| ) | ||
|
|
||
| def _overwrite_example( | ||
| self, example_origin, target, pack_name, example_name | ||
| ): | ||
| """Delete target and copy example.""" | ||
| shutil.rmtree(target) | ||
| shutil.copytree(example_origin, target) | ||
| self.console.print( | ||
| f"[green]Overwriting example '{pack_name}/{example_name}'.[/]" | ||
| ) | ||
|
|
||
| def _copy_missing_files(self, example_origin, target): | ||
| """Copy only files and directories that are missing in the | ||
| target.""" | ||
| for example_item in example_origin.rglob("*"): | ||
| rel_path = example_item.relative_to(example_origin) | ||
| target_item = target / rel_path | ||
| if example_item.is_dir(): | ||
| target_item.mkdir(parents=True, exist_ok=True) | ||
| elif example_item.is_file() and not target_item.exists(): | ||
| target_item.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(example_item, target_item) | ||
|
|
||
| def _copy_new_example( | ||
| self, example_origin, target, pack_name, example_name | ||
| ): | ||
| shutil.copytree(example_origin, target) | ||
| self.console.print( | ||
| f"[green]Copied example '{pack_name}/{example_name}'.[/]" | ||
| ) | ||
|
|
||
| def _resolve_pack_file(self, identifier: Union[str, Path]) -> Path: | ||
| """Resolve a pack identifier to an absolute .txt path. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,65 +0,0 @@ | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from diffpy.cmi import cli | ||
|
|
||
|
|
||
| def test_map_pack_to_examples_structure(): | ||
| """Test that map_pack_to_examples returns the right shape of | ||
| data.""" | ||
| actual = cli.map_pack_to_examples() | ||
| assert isinstance(actual, dict) | ||
| for pack, exdirs in actual.items(): | ||
| assert isinstance(pack, str) | ||
| assert isinstance(exdirs, list) | ||
| for ex in exdirs: | ||
| assert isinstance(ex, str) | ||
| # Check for known packs | ||
| assert "core" in actual.keys() | ||
| assert "pdf" in actual.keys() | ||
| # Check for known examples | ||
| assert ["linefit"] in actual.values() | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "input_valid_str", | ||
| [ | ||
| "core/linefit", | ||
| "pdf/ch03NiModelling", | ||
| ], | ||
| ) | ||
| def test_copy_example_success(tmp_path, input_valid_str): | ||
| """Given a valid example format (<pack>/<ex>), test that its copied | ||
| to the temp dir.""" | ||
| os.chdir(tmp_path) | ||
| actual = cli.copy_example(input_valid_str) | ||
| expected = tmp_path / Path(input_valid_str).name | ||
| assert expected.exists() and expected.is_dir() | ||
| assert actual == expected | ||
|
|
||
|
|
||
| def test_copy_example_fnferror(): | ||
| """Test that FileNotFoundError is raised when the example does not | ||
| exist.""" | ||
| with pytest.raises(FileNotFoundError): | ||
| cli.copy_example("pack/example1") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "input_bad_str", | ||
| [ | ||
| "", # empty string | ||
| "/", # missing pack and example | ||
| "corelinefit", # missing slash | ||
| "linefit", # missing pack and slash | ||
| "core/", # missing example | ||
| "/linefit", # missing pack | ||
| "core/linefit/extra", # too many slashes | ||
| ], | ||
| ) | ||
| def test_copy_example_valueerror(input_bad_str): | ||
| """Test that ValueError is raised when the format is invalid.""" | ||
| with pytest.raises(ValueError): | ||
| cli.copy_example(input_bad_str) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be introducing a new dependency. Is it worth adding the bloat for this? Why not just print? If we can be convinced it is worth it we probably have to add rich to all the dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbillinge I was planning on using it for the
print_infofunction (see image). it helps with separation. On second thought, I think we could also add it using ANSI color codes and get rid of the dependency. I'll do that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could use rich if it is a nice package, but it is a bit like using pathlib.Path, if we like it, we may want to kind of start using it as a group standard. We could even choose nice colors!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbillinge I agree! here's examples of what the syntax would look like with and without rich.
with rich:
without rich:
There's probably a way to print in bold with ANSI code too. I think
richhas preset colors that are commonly used/are nice but we could probably add a config file to change these if we wanted. what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind about the syntax, it is more whether we want to go there at all. Let's just do a print without any colors for now and merge the PR. We can make an issue to add rich formatting and have a discussion about it in the broader group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay done