Eliminate dependence on pyyaml-include by adapting Butler loader #89
Conversation
… it is an absolute path. 2) Otherwise it is interpreted as relative to current file (not necessarily the same as top-level file)
skycatalogs/utils/config_utils.py
Outdated
| return result | ||
|
|
||
| else: | ||
| print("Error:: unrecognised node type in !include statement", |
There was a problem hiding this comment.
Can a logger be used instead of a print statement here and below so that the output level can be controlled?
| stream : text io stream | ||
| The stream to parse. | ||
|
|
||
| This code was adapted from the LSST Science Pipelines Butler. |
There was a problem hiding this comment.
It would be useful to give a more complete reference to the LSST code, e.g., a url pointing to the daf_butler code.
| else: | ||
| actual_path = os.path.join(self._current_dir, filepath) | ||
| print("Opening YAML file via !include: %s", actual_path) | ||
|
|
There was a problem hiding this comment.
This formatting works for a logger message, but a print statement needs to have
print("Opening YAML file via !include: %s" % actual_path)
skycatalogs/utils/config_utils.py
Outdated
| """ | ||
| def __init__(self, filestream): | ||
| super().__init__(filestream) | ||
| self._root = os.path.dirname(filestream.name) |
There was a problem hiding this comment.
Are self._root and self._current_dir both needed? The base class doesn't seem to have a _root attribute, so both of these don't seem to be needed.
There was a problem hiding this comment.
That may be left over from earlier behavior, where all include paths were evaluated relative to initial file. I changed that to be relative to file with the reference. Will look into it.
Indeed, it's not used anywhere.
skycatalogs/utils/config_utils.py
Outdated
| old_current = self._current_dir | ||
| self._current_dir = actual_path | ||
| # Read all the data from the resource | ||
| with open(actual_path) as f: | ||
| content = yaml.load(f, YamlIncludeLoader) | ||
| self._current_dir = old_current |
There was a problem hiding this comment.
You don't need to do any special handling of self._current_dir here since it's set per-instance of YamlIncludeLoader. So this code can just be
# Read all the data from the resource
with open(actual_path) as f:
content = yaml.load(f, YamlIncludeLoader)
The other lines don't have any net effect, otherwise it would have been necessary to set self._current_dir = os.path.dirname(actual_path) on line 79.
JoanneBogart
force-pushed
the
u/jrbogart/yaml-include-class
branch
from
69a818e
to
c746f8f
Compare
Adapted the Butler loader, eliminating its use of lsst.resources so there are no dependencies on LSST Science Pipelines.
Moved yaml parsing code into config_utils.py