Skip to content

Comments

Deprecrate legacy and seperate example galleries#250

Open
samaloney wants to merge 8 commits intosunpy:mainfrom
samaloney:deprecate-legacy
Open

Deprecrate legacy and seperate example galleries#250
samaloney wants to merge 8 commits intosunpy:mainfrom
samaloney:deprecate-legacy

Conversation

@samaloney
Copy link
Member

@samaloney samaloney commented Feb 3, 2026

PR Description

  • Deprecate legacy, add warning on module import and add warnings call outs in the documentation.
  • Split gallery into Example and Legacy sections

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR deprecates the sunkit_spex.legacy module and reorganizes the documentation by splitting the example gallery into separate "Examples" and "Legacy Examples" sections.

Changes:

  • Added deprecation warning to sunkit_spex.legacy module that triggers on import
  • Split example gallery into main examples and legacy examples directories
  • Updated file paths in legacy examples to account for new directory structure
  • Updated documentation configuration and intersphinx mappings

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sunkit_spex/legacy/init.py Adds deprecation warning using SunpyDeprecationWarning when the legacy module is imported
pytest.ini Adds filter to ignore the legacy module deprecation warning during tests
pyproject.toml Replaces pypandoc dependency with joblib for sphinx-gallery parallel execution
examples/legacy/*.py Multiple legacy example files with updated file paths from ./ to ../../ to reflect new directory structure
examples/legacy/README.txt Adds README with deprecation warning for legacy examples section
docs/reference/legacy.rst Updates deprecation warning message to be more specific about version and timeline
docs/how_to/index.rst Adds legacy gallery section to documentation table of contents
docs/conf.py Modernizes import statement, updates intersphinx URLs, and enables parallel execution for sphinx-gallery
changelog/250.doc.rst Documents gallery split
changelog/250.deprecation.rst Documents legacy module deprecation
Comments suppressed due to low confidence (1)

examples/legacy/fitting_RHESSI_spectra.py:363

  • The save filename path is inconsistent with the data file paths. Data files use ../../rhessi/ to go two levels up, but the save filename uses ../ which only goes one level up. This should be ../../sunxspexRhessiSpectralFitting.pickle to be consistent with the directory structure and match the pattern used in other examples like fitting_STIX_spectra.py (line 205).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@KriSun95 KriSun95 left a comment

Choose a reason for hiding this comment

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

Other than the comment about multiple links it LGTM!

Happy to approve if the weird link issue is only on my machine too.

samaloney and others added 3 commits February 13, 2026 15:46
@samaloney
Copy link
Member Author

@KriSun95
Copy link
Collaborator

I didn't realise we could access the docs build from the tests in the PR 🤯

The sidebar menu now only shows the link to the Example Gallery. I got that behaviour when I just removed the link to ../generated/gallery/legacy/index in the docs/how_to/index.rst file instead of doing the :hidden: thing I suggested.

I suppose my question would be: do we want the behaviour that the legacy gallery is slightly more hidden? I could be convinced either way to be fair.

@samaloney
Copy link
Member Author

samaloney commented Feb 17, 2026

Yea it's because the legacy gallery is a subgallery of the example gallery. I didn't want to have to create a 2nd subgallery as I'd have to name it Current or something. So I've used the approach @KriSun95 suggested for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants