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

Q-Range slider fix for linear fits #1943

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Q-Range slider fix for linear fits #1943

merged 1 commit into from
Nov 5, 2021

Conversation

krzywon
Copy link
Contributor

@krzywon krzywon commented Oct 29, 2021

This fixes the issue noted in slack regarding an error when running a linear fit. This same error was also thrown when closing any plot with q-range sliders.

The signal used to link the GUI input to the sliders was changed from textChanged to editingFinished, but the disconnection was not changed to match.

@butlerpd
Copy link
Member

humm... probably that slackard @butlerpd approving without really testing properly. I'll have a word with him about disciplined and professional 😆

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

This fixes the issue both for linear fits and for closing the charts.
The code is OK.

@wpotrzebowski
Copy link
Contributor

wpotrzebowski commented Oct 29, 2021

Thanks @krzywon! I will test on Mac

@wpotrzebowski
Copy link
Contributor

ready for testing on OSX

@wpotrzebowski
Copy link
Contributor

ready for testing on Win

@wpotrzebowski
Copy link
Contributor

ready for testing on OSX

@wpotrzebowski
Copy link
Contributor

The qrange bug is indeed fixed. I am not sure if range on the Linear Scale works correctly though. First of all it doesn't seem to be editable (or I don't know how to activate it) and secondly as soon as I start changing values in the log range, linear range gives strange values (outside q range).

Screen Shot 2021-10-30 at 07 11 31
.

@krzywon
Copy link
Contributor Author

krzywon commented Nov 1, 2021

@wpotrzebowski, a lot of what you're seeing is documented in #1415, but please add more there if what you're seeing is different. I don't plan to go beyond fixing the error thrown in this PR.

@rozyczko
Copy link
Member

rozyczko commented Nov 3, 2021

Can this be merged, please?

@wpotrzebowski
Copy link
Contributor

ready for testing on OSX

@wpotrzebowski wpotrzebowski merged commit ca2f0e3 into main Nov 5, 2021
@wpotrzebowski wpotrzebowski deleted the linear-fit-fix branch November 5, 2021 06:17
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.

4 participants