ENH: add wavedecn and waverecn functions #67

grlee77 · 2015-07-24T02:48:22Z

adds n-dimensional variants of wavedec2 and waverec2.
This was split off from the now closed #52

rgommers · 2015-07-24T05:47:44Z

pywt/multilevel.py

+        Signal extension mode, see MODES (default: 'sym')
+    level : int, optional
+        Decomposition level. If level is None (default) then it will be
+        calculated using the ``dwt_max_level`` function.


Would be good to document here that level needs to be >=0

kwohlfahrt · 2015-07-24T12:24:44Z

Looks good at a quick glance. This is one way of working around #54. Personally, I think idwtn should be strict in what input shapes it accepts, and not silently truncate it's input, but for a multilevel transform this is the correct thing to do. Should we then close #54, or still change the behaviour of idwtn?

grlee77 · 2015-07-24T22:17:15Z

If you want to add documentation on why the behaviour in #54 is expected and mention this as a workaround, I would be okay with that. I don't really know which behaviour is preferable for idwtn,

grlee77 · 2015-08-14T23:26:42Z

This was rebased using current master. A major overhaul of the tests was done in the last commit to make them more comprehensive and consistent across all dimensions.

grlee77 · 2015-08-14T23:29:36Z

@kwohlfahrt can you check if the updated implementation of waverecn here looks reasonable to you. I added a utility function called _match_coeff_dims to get things to work now that take is no longer an argument of idwtn.

kwohlfahrt · 2015-09-04T10:50:49Z

@grlee77 sorry, I completely missed that I'd been tagged here. The idea looks clean, however in idwtn we allow (and silently ignore) the dictionary to contain keys which are not just combinations of 'a' and 'd', and values which are None. _match_coeff_dims does not handle this at the moment.

There are a few edge cases (e.g. one level being {}) that are not handled as well. I'll write tests and fixes for as many as I can catch and open a PR on your repo. Sorry again for taking so long to respond.

grlee77 · 2015-10-26T14:11:01Z

I merged @kwolfahrt's code to handle missing or invalid coefficients, but please do not merge just yet. I still want to add a warning in the case of missing coefficients or unexpected extra keys in the detail coefficient dictionary and an exception if any coefficients have the wrong shape.

grlee77 · 2015-10-26T16:15:44Z

@kwohlfahrt
Please review the most recent commit. I added an enable_warnings argument to _fix_coeffs that raises warnings for the following two cases:

any detail coefficient arrays are None
any keys in the detail coefficient dictionary have invalid names

waverecn itself now raises a warning if any of the detail coefficient dictionaries are empty.

idwtn also uses fix_coeffs, but I did not enable the warnings there yet. Do you think we should do so? If so, perhaps enable_warnings should be False in the waverecn call to _fix_coeffs so we don't raise the same warnings twice (or the call to _fix_coeffs could be removed from waverecn since idwtn is going to call it again anyways).

kwohlfahrt · 2015-11-05T14:49:16Z

Sorry for the delay, here are my thoughts:

I think it's probably better to make this an exception, and say all values must be array_like
- If we're worried about compatibility, a deprecation warning for a release cycle is probably OK. Though iirc we were breaking back-compat so no need to fuss. I'd catch the exception and raise the warning instead, just to get the code for the future written now.
Maybe this too? I'm pretty on the fence about it.

I'm not sure it's necessary to warn if any detail coefficient dictionaries are empty. It's an extension of allowing any other number of coefficients to be missing, so it's not really unusual enough to cause a warning.

I would also enable the warnings/exceptions for idwtn. If it is used everywhere, I don't think the enable_warnings flag should exist, it should just be the behaviour of the function.

grlee77 · 2015-11-20T15:44:05Z

@kwohlfahrt
see how this looks and let me know if there is a cleaner way to do this.

I removed the warning about empty coefficient dictionaries as suggested.

I now raise a DeprecationWarning if any dictionary entries are None or have an invalid key name. This will become a ValueError after the deprecation period.

kwohlfahrt · 2015-11-26T14:08:52Z

_fix_coeffs should also check if there are (valid) keys that are not the same length. A dictionary containing 'aa' and 'aaa' should be an error.

Given that we 'broke' idwtn already in #125, I don't think the deprecation warning is necessary any more, but up to you.

Otherwise looks good!

grlee77 · 2015-12-15T00:34:14Z

@kwohlfahrt
any other comments here? If not, feel free to merge it

kwohlfahrt · 2015-12-15T17:28:22Z

pywt/_multilevel.py

+    coeffs_list.append(a)
+    coeffs_list.reverse()
+
+    if len(coeffs_list) < level + 1:


Looking at the lines above, this test shouldn't ever fail (and it doesn't break any tests if removed).

reorganize tests into a logical order and use uniform tolerances across all tests. merge the separate *_complex tests into the dtype *_accuracies tests to avoid duplicating functionality add comprehensive mode and wavelet type tests to the 1D and 2D cases to match those previously existing for nD added dec.slow or PYWT_XSLOW decorators where needed. In the future if dwt2/idwt2 are refactored to use dwtn/idwtn the PYWT_XSLOW decorator can be removed.

Correctly handle missing coefficients in waverecn, as they are treated in idwtn.

Previously, empty levels were discarded, they should not be.

…rrays during waverecn

…efficients. Temporarily catch the Exception and raise a warning instead to allow for a deprecation period

…ax_level

…ning for invalid keys was removed

… the key lengths

…ore calling idwtn

…nd restore missing import in test_multidim.py

… are fewer than the number of array dimensions.

…s_modes now that the underlying code is much faster

grlee77 · 2015-12-16T20:50:33Z

@kwohlfahrt: still waiting on Travis, but I addressed the issue above and added a couple new tests of the axes arguments to dwtn, idwtn.

kwohlfahrt · 2015-12-17T15:57:52Z

pywt/_multilevel.py

+    """
+
+    if not isinstance(coeffs, (list, tuple)):
+        raise ValueError("Expected sequence of coefficient arrays.")


I don't think this check is necessary - other sequences should work fine.

kwohlfahrt · 2015-12-17T16:05:33Z

Apart from the isinstance check, looks good to me. I don't think we should reject other types of sequence unless there's a particular reason for this.

…t of waverecn

grlee77 · 2015-12-17T16:16:20Z

okay. should be good to go now

ENH: add wavedecn and waverecn functions

grlee77 added this to the v0.4.0 milestone Jul 24, 2015

rgommers added the enhancement label Jul 24, 2015

rgommers reviewed Jul 24, 2015
View reviewed changes

kwohlfahrt modified the milestones: v0.3.0, v0.4.0 Jul 24, 2015

grlee77 force-pushed the wavedecn_v2 branch from 70648a2 to 5e9f162 Compare July 25, 2015 21:35

kwohlfahrt mentioned this pull request Jul 26, 2015

idwtn doesn't tolerate unequal shapes in coefficients #54

Closed

grlee77 mentioned this pull request Jul 29, 2015

Interface improvement: consistent return format between swt and swt2 #81

Closed

grlee77 force-pushed the wavedecn_v2 branch from 5e9f162 to 44d9551 Compare August 14, 2015 23:24

grlee77 force-pushed the wavedecn_v2 branch from f35925d to d9a3f9a Compare October 26, 2015 16:08

grlee77 force-pushed the wavedecn_v2 branch from b36a5b0 to 6edb87f Compare November 20, 2015 15:12

grlee77 mentioned this pull request Nov 20, 2015

planning for v0.4.0 release #128

Closed

grlee77 force-pushed the wavedecn_v2 branch 2 times, most recently from 252645f to a01939d Compare November 25, 2015 23:44

kwohlfahrt reviewed Dec 15, 2015
View reviewed changes

grlee77 and others added 18 commits December 16, 2015 14:18

MAINT: update wavedecn to use the new idwtn/dwtn implementations

6d10891

TST: add more thorough roundtrip tests for all wavelets and modes

2847532

TST: add dtype tests and complex test for nd case

20f429f

MAINT: pep8 fixes

79f9ef0

BUG/TST: Handle invalid coeffs in waverecn

b1ab5f1

Correctly handle missing coefficients in waverecn, as they are treated in idwtn.

BUG: Fix handling of empty levels in waverecn

c6d174d

Previously, empty levels were discarded, they should not be.

ENH: Enable warnings for invalid keys or missing detail coefficient a…

2d98b74

…rrays during waverecn

MAINT: idwtn/waverecn now raise an exception on missing or invalid co…

025e032

…efficients. Temporarily catch the Exception and raise a warning instead to allow for a deprecation period

MAINT: remove warning on empty coefficient dictionary and clean up tests

dc35b2f

TST: test_compare_downcoef_coeffs: only run tests for levels <= dwt_m…

1edb3d7

…ax_level

MAINT: PEP8

08507e7

ENH: _fix_coeffs also checks for mismatched key names. DeprecationWar…

6462f1c

…ning for invalid keys was removed

MAINT: remove useless test from wavedecn

d63996c

MAINT: remove unused imports for warnings that are no longer used

bf08677

ENH/TST: also check that the idwtn coefficient array dimensions match…

6005987

… the key lengths

BUG: idwt2 must drop any coefficient keys where the value is None bef…

36afd9d

…ore calling idwtn

BUG: move coefficient dimension checks from _fix_coeffs to wavedecn a…

a9eea82

…nd restore missing import in test_multidim.py

grlee77 force-pushed the wavedecn_v2 branch from d6842ec to a9eea82 Compare December 16, 2015 20:33

grlee77 added 2 commits December 16, 2015 15:40

TST: add a couple of tests where the number of axes to be transformed…

e11f050

… are fewer than the number of array dimensions.

TST/MAINT: remove PYWT_XSLOW decorator from test_waverec2_all_wavelet…

3ddd8b5

…s_modes now that the underlying code is much faster

grlee77 mentioned this pull request Dec 16, 2015

Autogenerate function API docs #139

Merged

kwohlfahrt reviewed Dec 17, 2015
View reviewed changes

MAINT: remove unnecessarily restrictive test on sequence type at star…

af75758

…t of waverecn

kwohlfahrt pushed a commit that referenced this pull request Dec 17, 2015

Merge pull request #67 from grlee77/wavedecn_v2

2a139a3

ENH: add wavedecn and waverecn functions

kwohlfahrt merged commit 2a139a3 into PyWavelets:master Dec 17, 2015

grlee77 deleted the wavedecn_v2 branch December 29, 2015 05:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: add wavedecn and waverecn functions #67

ENH: add wavedecn and waverecn functions #67

grlee77 commented Jul 24, 2015

rgommers Jul 24, 2015

kwohlfahrt commented Jul 24, 2015

grlee77 commented Jul 24, 2015

grlee77 commented Aug 14, 2015

grlee77 commented Aug 14, 2015

kwohlfahrt commented Sep 4, 2015

grlee77 commented Oct 26, 2015

grlee77 commented Oct 26, 2015

kwohlfahrt commented Nov 5, 2015

grlee77 commented Nov 20, 2015

kwohlfahrt commented Nov 26, 2015

grlee77 commented Dec 15, 2015

kwohlfahrt Dec 15, 2015

grlee77 commented Dec 16, 2015

kwohlfahrt Dec 17, 2015

kwohlfahrt commented Dec 17, 2015

grlee77 commented Dec 17, 2015

ENH: add wavedecn and waverecn functions #67

ENH: add wavedecn and waverecn functions #67

Conversation

grlee77 commented Jul 24, 2015

rgommers Jul 24, 2015

Choose a reason for hiding this comment

kwohlfahrt commented Jul 24, 2015

grlee77 commented Jul 24, 2015

grlee77 commented Aug 14, 2015

grlee77 commented Aug 14, 2015

kwohlfahrt commented Sep 4, 2015

grlee77 commented Oct 26, 2015

grlee77 commented Oct 26, 2015

kwohlfahrt commented Nov 5, 2015

grlee77 commented Nov 20, 2015

kwohlfahrt commented Nov 26, 2015

grlee77 commented Dec 15, 2015

kwohlfahrt Dec 15, 2015

Choose a reason for hiding this comment

grlee77 commented Dec 16, 2015

kwohlfahrt Dec 17, 2015

Choose a reason for hiding this comment

kwohlfahrt commented Dec 17, 2015

grlee77 commented Dec 17, 2015