Prevent validation of empty tree when AsdfFile is initialized #794
Conversation
Codecov Report
@@ Coverage Diff @@
## master #794 +/- ##
=======================================
Coverage 94.00% 94.00%
=======================================
Files 43 43
Lines 5054 5056 +2
=======================================
+ Hits 4751 4753 +2
Misses 303 303
Continue to review full report at Codecov.
|
eslavich
force-pushed
the
eslavich-skip-validating-empty-tree
branch
from
d36cdba
to
7bedc82
Compare
| self.tree = {} | ||
| # Bypassing the tree property here, to avoid validating | ||
| # an empty tree. | ||
| self._tree = AsdfObject() |
There was a problem hiding this comment.
Might it be better to just pass tree to the setter for self.tree, and have the setter handle None? This whole block seems like it should be handled by the setter, not just on __init__.
There was a problem hiding this comment.
When you say whole block, is that the whole if/elif/else? I don't think we want the setter to handle copy-constructing from another AsdfFile, since that requires messing around with blocks and copying other AsdfFile internal variables besides the tree.
I have a slight preference for keeping the commented line as-is, since users can already pass {} to set an empty tree, and it seems tidy for there to be only one way to do it.
I noticed this while playing with #792 -- some validation was still occurring despite the new flag. Turns out when
AsdfFileis initialized, we're setting thetreeproperty to an empty dict, which results in a validation pass over that object. This PR bypasses that validation by setting_treedirectly.This PR also fixes a minor bug when opening a file containing only blocks, and fixes some tests that relied upon a side-effect of the unnecessary validation pass.