DM-25030 make an import butler subcommand #293
Conversation
| # better than hard coding the function names into these conversion | ||
| # functions. An extension of the 'cli/resources.yaml' file (as is currently | ||
| # used in obs_base) might be a good way to do it. | ||
| if functionName == "butler_import": |
There was a problem hiding this comment.
One other option that may be less prone to scaling problems could be to define a dict that maps from "import" to "butler_import" and then automatically generate the inverse dict. Then in these two functions you do the equivalent of:
functionName = funcToCommand.get(functionName, functionName)(I understand that my dict name there is non-optimal)
There was a problem hiding this comment.
I thought about making some kind of data structure like that, but if I'm going to do even that much work I wanted to take a moment to figure out the best way to solve the problem. I think it might involve the plugin system, but I didn't want to spent that much time on it or add complexity, yet.
|
|
||
| Parameters | ||
| ---------- | ||
| required : bool, optional |
There was a problem hiding this comment.
Backticks around the bool are needed for numpydoc.
|
|
||
| Returns | ||
| ------- | ||
| butler : Butler |
| directory : `str`, or None | ||
| Directory containing dataset files. If `None`, all file paths must be | ||
| absolute. | ||
| export_file : `str`, optional |
There was a problem hiding this comment.
This doesn't look like an optional parameter in the sense that you don't need to specify it. You do need to specify it but it can be None.
| filename=export_file, | ||
| transfer=transfer, | ||
| format="yaml") | ||
| return butler |
There was a problem hiding this comment.
I don't see anything using this return parameter. Why does it exist?
There was a problem hiding this comment.
leftover from trying to fix a unit test issue that ended up not being an issue. Not needed, will remove.
tests/test_cliCmdImport.py
Outdated
| case below. | ||
| """ | ||
| with self.runner.isolated_filesystem(): | ||
| f = open("output.yaml", "w") |
There was a problem hiding this comment.
What is the purpose of opening this file? It doesn't seem to be used anywhere.
There was a problem hiding this comment.
also leftover from addressing a unit test issue, the fix ended up becoming ExportFileCase. Will remove.
| Parameters | ||
| ---------- | ||
| repo : `str` | ||
| URI to the location of the repo. |
There was a problem hiding this comment.
URI to the location of the repo or URI to a config file describing the repo and its location.
| @@ -84,31 +84,33 @@ def test_loadAndExecutePluginCommand(self): | |||
| runner = click.testing.CliRunner() | |||
| with command_test_env(runner): | |||
| result = runner.invoke(butler.cli, "command-test") | |||
| self.assertEqual(result.exit_code, 0, result.output) | |||
| self.assertEqual(result.exit_code, 0, f"output: {result.output} exception: {result.exception}") | |||
There was a problem hiding this comment.
It seems like you have fixed the commands caching so I think that means you can remove the setUp above.
n8pease
force-pushed
the
tickets/DM-25030
branch
3 times, most recently
from
bfb4125
to
6884e08
Compare
tickets/DM-25030-transfer
No description provided.