Asdftool edit

[kmacdonald-stsci]

I have added an 'edit' subcommand to the asdftool. It separates the ASDF markings and YAML text into a separate file to be edited, using

asdftool -e -f <ASDF file> -o <YAML file name>

Once editing the YAML is complete, use

asdftool -s -f <edited YAML file> -o <ASDF to save YAML to>

There may be a gotcha when saving the edited YAML back to ASDF file. If the edited YAML contains for characters than the number of characters before the binary blocks in the ASDF file, an error message is returned.

[perrygreenfield]

Finally, I have a vague sense that some of this code is similar to what is in AsdfFile and there is going to be unnecessary duplication. On the other hand, what is in AsdfFile doesn't quite meet the needs of this and I think it would need a little reworking to be useful for this context. I wouldn't expect that you would have to do that reworking. Perhaps some future refactoring?

[eslavich]

This is a good start, in general I think we should take more opportunities for code sharing between edit.py and asdf.py/block.py.

Also, just a heads up, we're going to need to figure out some pytest tests for this, but that should wait until we're all agreed on the implementation.

[eslavich]

There is a header struct-like thing in the Block class that we can reuse here.

[eslavich]

This function on the whole seems like it can be shared with asdf.py.

[perrygreenfield]

I don't think the request to bypass validation on opening should hold up acceptance.

[eslavich]

The streaming block case is a serious problem, and IMO we should still change that padding character. Can you also look back over the comments from previous rounds of review? GitHub marks them as "outdated" when the file changes but some of them haven't actually been resolved.

[jdavies-st]

This looks really good. Just a couple specific comments below.

And a general comment - it looks like there are a lot of print() statements here when handling errors, and this is the case with a bunch of the other asdftool subcommands as well. It might be better to use the logging module I think, but perhaps that ship has sailed. =)

[eslavich]

@kmacdonald-stsci I discovered a bug in testing the tool, kind of a silly case but I think we should handle it:

• Create an ASDF file with no binary blocks:

import asdf

with asdf.AsdfFile() as af:
  af["foo"] = "bar"
  af.write_to("test.asdf")

• Use asdftool edit -e to split the file
• In all cases of editing the YAML (equal, shorter, or longer file size) asdftool edit -s raises the following exception:

Traceback (most recent call last):
  File "/Users/eslavich/anaconda3/envs/asdftool-edit/bin/asdftool", line 33, in <module>
    sys.exit(load_entry_point('asdf', 'console_scripts', 'asdftool')())
  File "/Users/eslavich/repos/asdf/asdf/commands/main.py", line 87, in main
    sys.exit(main_from_args(args))
  File "/Users/eslavich/repos/asdf/asdf/commands/main.py", line 70, in main_from_args
    result = args.func(args)
  File "/Users/eslavich/repos/asdf/asdf/commands/edit.py", line 91, in run
    return edit(args)
  File "/Users/eslavich/repos/asdf/asdf/commands/edit.py", line 649, in edit
    return save_func(args.fname, args.oname)
  File "/Users/eslavich/repos/asdf/asdf/commands/edit.py", line 633, in save_func
    loc = find_first_block(oname)
  File "/Users/eslavich/repos/asdf/asdf/commands/edit.py", line 432, in find_first_block
    reader.read()  # Read to the beginning of the first binary block.
  File "/Users/eslavich/repos/asdf/asdf/generic_io.py", line 224, in read
    raise ValueError("{0} not found".format(self._delimiter_name))
ValueError: First binary block not found

ASDF files without blocks will become more common after we change the ndarray auto-inline threshold to 100, which I think I heard is coming soon.

[eslavich]

I think this turned out great! Thanks for bearing with all the review. I suspect we'll want the next version (that launches an editor automatically) to be able to work with files without binary blocks, but the current solution is fine for now.