-
Notifications
You must be signed in to change notification settings - Fork 37
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
Include params in HDF5 output #142
Changes from 16 commits
ba9ff2f
65ca6f4
651ba7c
0c72713
2bedb3c
e4137b5
51873be
ca8fb9b
5fad0c8
a48430b
0ea18ec
abde1bb
e1c069d
a7e3c8e
fd0a8cd
948b1c2
3ead01f
b0d6601
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 |
---|---|---|
|
@@ -87,7 +87,7 @@ def static_warning(self, *args, **kwargs): | |
self.load_from = static_warning | ||
|
||
@staticmethod | ||
def compute(data, min_value=-np.inf, min_delta=0, min_npix=0, | ||
def compute(data, min_value="min", min_delta=0, min_npix=0, | ||
is_independent=None, verbose=False, neighbours=None, wcs=None): | ||
""" | ||
Compute a dendrogram from a Numpy array. | ||
|
@@ -96,9 +96,10 @@ def compute(data, min_value=-np.inf, min_delta=0, min_npix=0, | |
---------- | ||
data : :class:`numpy.ndarray` | ||
The n-dimensional array to compute the dendrogram for | ||
min_value : float, optional | ||
min_value : float or "min", optional | ||
The minimum data value to go down to when computing the | ||
dendrogram. Values below this threshold will be ignored. | ||
dendrogram. Values below this threshold will be ignored. Defaults | ||
to the minimum value in the data. | ||
min_delta : float, optional | ||
The minimum height a leaf has to have in order to be considered an | ||
independent entity. | ||
|
@@ -128,9 +129,9 @@ def compute(data, min_value=-np.inf, min_delta=0, min_npix=0, | |
that has been padded with one element along each edge. | ||
|
||
wcs : WCS object, optional | ||
A WCS object that describes `data`. This is used in the | ||
interactive viewer to properly display the data's coordinates | ||
on the image axes. (Requires that `wcsaxes` is installed; see | ||
A WCS object that describes `data`. This is used in the | ||
interactive viewer to properly display the data's coordinates | ||
on the image axes. (Requires that `wcsaxes` is installed; see | ||
http://wcsaxes.readthedocs.org/ for install instructions.) | ||
|
||
|
||
|
@@ -160,6 +161,10 @@ def compute(data, min_value=-np.inf, min_delta=0, min_npix=0, | |
is_independent = pruning.all_true(tests) | ||
neighbours = neighbours or Dendrogram.neighbours | ||
|
||
# Default min_val to the minimum in the data | ||
if min_value == "min": | ||
min_value = np.min(data[np.isfinite(data)]) - 1 | ||
|
||
self = Dendrogram() | ||
self.data = data | ||
self.n_dim = len(data.shape) | ||
|
@@ -463,6 +468,23 @@ def __eq__(self, other): | |
if not (self.data == other.data).all(): | ||
return False | ||
|
||
if self.params['min_value'] != other.params['min_value']: | ||
return False | ||
|
||
self_params = self.params | ||
other_params = other.params | ||
|
||
self_params.pop('min_value') | ||
other_params.pop('min_value') | ||
|
||
# Don't compare params if one has not been set (ie. = 0) | ||
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. I don't like this comment, since I think someone could explicitly set a param to 0. I'm fine with the logic since it's getting the tests to pass and 0 might usually mean "not explicitly set", but this comment should indicate that this is a small hack/workaround |
||
for key in self_params.keys(): | ||
if self_params[key] == 0 or other_params[key] == 0: | ||
continue | ||
|
||
if self_params[key] != other_params[key]: | ||
return False | ||
|
||
# structures should have the same extent, | ||
# but idx values need not be identical. This | ||
# tests the index map for that | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,13 @@ def dendro_export_fits(d, filename): | |
except AttributeError: | ||
primary_hdu = fits.PrimaryHDU() | ||
|
||
primary_hdu.header["MIN_NPIX"] = (d.params['min_npix'], | ||
"Minimum number of pixels in a leaf.") | ||
primary_hdu.header["MIN_DELT"] = (d.params['min_delta'], | ||
"Minimum branch height.") | ||
primary_hdu.header["MIN_VAL"] = (d.params['min_value'], | ||
"Minimum intensity value.") | ||
|
||
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. Should these parameters be in the fits header instead of a new extension? That would seem more standard to me 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. Yes, I agree, these belong in the header (I think of the primary HDU) 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. Thanks, the table seemed quite clunky. I moved them to the header. |
||
hdus = [primary_hdu, | ||
fits.ImageHDU(d.data), | ||
fits.ImageHDU(d.index_map), | ||
|
@@ -59,7 +66,11 @@ def dendro_import_fits(filename): | |
index_map = hdus[2].data | ||
newick = ''.join(chr(x) for x in hdus[3].data.flat) | ||
|
||
return parse_dendrogram(newick, data, index_map, wcs) | ||
params = {"min_npix": hdus[0].header['MIN_NPIX'], | ||
"min_value": hdus[0].header['MIN_VAL'], | ||
"min_delta": hdus[0].header['MIN_DELT']} | ||
|
||
return parse_dendrogram(newick, data, index_map, params, wcs) | ||
|
||
|
||
FITSHandler = IOHandler(identify=is_fits, | ||
|
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.
ooo careful here --
pop
permanently mutates the params dict of the parent dendrograms. Better to setself_params = self.params.copy(), other_params = other.params.copy()