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

Fix infinite toggle loop #202

Merged
merged 7 commits into from
Aug 3, 2020
Merged

Conversation

nmearl
Copy link
Contributor

@nmearl nmearl commented Jul 27, 2020

Resolves #185. Fixes an issue when manipulating the data collection that could result in an infinite loop occurring with the toggled data list items.

🚨NOTE FOR FUTURE REFERENCE🚨 Please, do not attempt to alter application state from within plugins! This can have obviously unintended consequences due to event ordering!

@nmearl nmearl requested review from javerbukh and rosteen July 27, 2020 18:35
Copy link
Collaborator

@rosteen rosteen left a comment

Choose a reason for hiding this comment

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

It doesn't look like moving the code in now in the app._on_data_deleted method out of the plugin solves the loop issue, I was still able to replicate the infinite loop behavior after deleting the relevant code from the model fitting plugin. Tested from a clean install.

EDIT: wow, I meant does not solve the loop issue, the original comment was thus rather confusing.

@nmearl
Copy link
Contributor Author

nmearl commented Jul 30, 2020

@rosteen, locally, does this solve the looping issue seen in #185 with the unit conversion plugin?

@rosteen
Copy link
Collaborator

rosteen commented Jul 30, 2020

No, I'm still able to replicate the looping with the unit conversion as well.

@nmearl
Copy link
Contributor Author

nmearl commented Jul 30, 2020

I cannot replicate with this PR -- are you testing with the Specviz Unit Conversion notebook?

@rosteen
Copy link
Collaborator

rosteen commented Jul 30, 2020

I was using the Specviz hack hour notebook. I had to change units 6 or 7 times before the flickering/looping started, but through persistence was able to break it.

Copy link
Contributor

@javerbukh javerbukh left a comment

Choose a reason for hiding this comment

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

Could not replicate bug in a clean conda environment even after about 10 tries (I am using Chrome for my browser, if that helps).

Copy link
Collaborator

@rosteen rosteen left a comment

Choose a reason for hiding this comment

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

Forgot to actually approve - I am still seeing odd behavior very occasionally, but I think at this point the root cause is my browser/local notebook behaving poorly rather than a code fix needed.

@nmearl nmearl merged commit b15d8f4 into spacetelescope:master Aug 3, 2020
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.

Switching between data in code causes infinite loop
3 participants