MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

MAINT: remove optional dependence on SciPy for FFT operations - #791

Merged
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy
Feb 3, 2025
Merged

MAINT: remove optional dependence on SciPy for FFT operations#791
rgommers merged 9 commits into
PyWavelets:mainfrom
agriyakhetarpal:drop/scipy

Conversation

@agriyakhetarpal

Copy link
Copy Markdown
Collaborator

Description

This PR removes SciPy as an optional dependency for PyWavelets, making NumPy the only dependency. This follows the discussion from #741 (comment).

Changes made

  • Removal of SciPy-related CI options, such as the SCIPY_MIN and USE_SCIPY environment variables and their corresponding jobs created from them, and removal of scipy as a nightly dependency that's installed from the SPNW Anaconda.org index
  • The pywt/data/create_dat.py helper script has been moved to the util/ directory outside the package's source tree.
    The README/manual, the installation guide, and the optional-dependencies table in pyproject.toml no longer mention SciPy as an optional dependency. At some other point, the optional dependencies could be populated with [test] or [docs].
  • PyWavelets now uses numpy.fft to calculate the continuous wavelet transform (through pywt/_cwt.py), as NumPy >=2 has been available for some while now. We could advise users about the performance regression (if they still have NumPy 1.x in their environment) through the release notes. I haven't spent time benchmarking whether SciPy is still faster or not.

Other references to SciPy, such as its intersphinx mapping, and PyWavelets's adoption of its code of conduct from SciPy's, mailing list links, a few code comments, and so on have been kept.

Question(s)

There is a "FIXME:" comment here:

pywt/pywt/_functions.py

Lines 80 to 102 in 281caa4

Examples
--------
>>> from pywt import Wavelet, integrate_wavelet
>>> wavelet1 = Wavelet('db2')
>>> [int_psi, x] = integrate_wavelet(wavelet1, precision=5)
>>> wavelet2 = Wavelet('bior1.3')
>>> [int_psi_d, int_psi_r, x] = integrate_wavelet(wavelet2, precision=5)
"""
# FIXME: this function should really use scipy.integrate.quad
iftype(wavelet) in (tuple, list):
msg= ("Integration of a general signal is deprecated "
"and will be removed in a future version of pywt.")
warnings.warn(msg, DeprecationWarning)
elifnotisinstance(wavelet, (Wavelet, ContinuousWavelet)):
wavelet=DiscreteContinuousWavelet(wavelet)
iftype(wavelet) in (tuple, list):
psi, x=np.asarray(wavelet[0]), np.asarray(wavelet[1])
step=x[1] -x[0]
return_integrate(psi, step), x
which has been pending for some while, as the NumPy implementation for the integration might not have been the ideal choice back then. If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do? The commit is twelve years old, though: ddda4dd

Additional context

N/A

Comment thread.github/workflows/tests.yml Outdated
@rgommersrgommers changed the title Remove optional dependence on SciPy for FFT operationsMAINT: remove optional dependence on SciPy for FFT operationsFeb 3, 2025
@rgommersrgommers added this to the v1.9.0 milestone Feb 3, 2025

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @agriyakhetarpal!

If we want to fix it, SciPy should stay as an optional dependency, and shouldn't be removed. What do you think we should do?

We'll just leave that alone, no need to take action. It may be useful in case someone starts looking at an accuracy issue with that functionality, but it seems unlikely to ever happen.

At some other point, the optional dependencies could be populated with [test] or [docs].

I'd prefer to not do that now. Maybe in a year or so the dependency groups functionality becomes commonplace, and then that's the more appropriate thing to use.

@rgommers
rgommers merged commit becef54 into PyWavelets:mainFeb 3, 2025
@agriyakhetarpal
agriyakhetarpal deleted the drop/scipy branch February 3, 2025 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@agriyakhetarpal@rgommers