Skip to content
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

Update notebooks to remove deprecated methods #192

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

rosteen
Copy link
Contributor

@rosteen rosteen commented Dec 4, 2023

Recent Jdaviz releases have deprecated (and finally removed) the app.get_data_from_viewer, app.get_subsets_from_viewer, and specviz.load_spectrum methods. This PR updates affected notebooks accordingly. I tagged a few relevant folks as reviewers, wasn't sure who all would want to see this.

@rosteen rosteen added the bug Something isn't working label Dec 4, 2023
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kecnry
Copy link
Member

kecnry commented Dec 5, 2023

print(cubeviz.app.data_collection)

I wonder if we need something more user-friendly at the helper level for this case (exploring what data exists in the app, rather than just extracting data where you already know the label, etc)? With what exists now, this probably makes sense for the notebook, though.

@rosteen
Copy link
Contributor Author

rosteen commented Dec 5, 2023

The notebook execution failures are due to needing UI interactions to define subsets before running those notebook cells, I assume those tests have never passed. And the pep8 failures appear unrelated to my changes.

@camipacifici
Copy link
Contributor

The ifu_optimal notebook is in another PR with edits from Patrick. Maybe Ricky's changes have already been addressed in Patrick's PR.

@rosteen
Copy link
Contributor Author

rosteen commented Dec 11, 2023

@haticekaratay I fixed the conflicts with main, I think this is good to go now. I left in two updates to Patrick's IFU notebook to better use current API methods.

Copy link
Collaborator

@haticekaratay haticekaratay left a comment

Choose a reason for hiding this comment

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

Thanks, Ricky, for your work. A follow-up ticket is created to fix the execution and styles that are out of the scope of this PR.

@haticekaratay haticekaratay merged commit 42da864 into spacetelescope:main Dec 11, 2023
5 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants