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

Options widget improvements #34

Merged
merged 2 commits into from Oct 7, 2015
Merged

Options widget improvements #34

merged 2 commits into from Oct 7, 2015

Conversation

astrofrog
Copy link
Member

@PennyQ @maxwelltsai - this tidied up the options widget a little and fixes #25

@PennyQ
Copy link
Contributor

PennyQ commented Oct 1, 2015

Thanks, this looks very great. I found that the py.test failed with AttributeError: 'QtVispyWidget' object has no attribute 'axes', maybe you could just footnote the last line in 'test_vispy_widget.py' first (which causes this error) and merge it, later we could try to write a better structured test unit.

@astrofrog
Copy link
Member Author

I just realized there is a change missing here, will commit later today (and that will fix error). I also want to make a few other improvements to the options widget, which I'll also make later today.

@astrofrog
Copy link
Member Author

I accidentally opened this from a branch I pushed to this repo instead of my repository... I'll delete the branch from this repo once it's merged.

@astrofrog astrofrog force-pushed the options-improvements branch 2 times, most recently from 2e317a2 to b563846 Compare October 2, 2015 14:49
@astrofrog
Copy link
Member Author

@PennyQ @maxwelltsai - I've done some significant refactoring to try and address both #25 and #35 as well as make sure things are better separated.

The options widget now includes a label giving the stretch value that auto-updates, and the ability to set that stretch value manually. In addition, the component of the data can now be selected, and the correct axis labels are used (instead of just RA/DEC/VEL).

The canvas-specific code setting e.g. camera and data parameters has been refactored and is now in the vispy widget rather than in the options widget. The options widget is kept as simple as possible and simply triggers a refresh from the data viewer when any options are updated. Before, the options widget had to go and modify the camera elements, which is not ideal.

The diff is going to be hard to read, but I encourage you to look at the code in the branch:

https://github.com/glue-viz/glue-3d-viewer/tree/options-improvements/vispy_volume

And also feel free to try out that branch to check if anything is broken (if you prefer, we can merge and then you can test it out, I'll leave that up to you).

@astrofrog
Copy link
Member Author

Screenshot of current options widget:

screen shot 2015-10-02 at 5 02 01 pm

The options widget now includes a label giving the stretch value that
auto-updates, and the ability to set that stretch value manually. In
addition, the component of the data can now be selected.

The canvas-specific code setting e.g. camera and data parameters has been
refactored and is now in the vispy widget rather than in the options widget.
The options widget is kept as simple as possible and simply triggers a
refresh from the data viewer when any options are updated.
@PennyQ
Copy link
Contributor

PennyQ commented Oct 6, 2015

@astrofrog This looks so great, very looking forward to seeing it be merged.
Maxwell and I have been worked on the #30 for sometime but it still has some problems. The test Python file is listed here https://github.com/PennyQ/Test/blob/master/volume_test_axis.py and any advice would be very helpful.

@astrofrog
Copy link
Member Author

@PennyQ - Thanks for reviewing! Will merge now - I hope this doesn't cause to many conflicts for your changes relating to the axes!

astrofrog added a commit that referenced this pull request Oct 7, 2015
@astrofrog astrofrog merged commit b65195b into master Oct 7, 2015
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.

Show value of stretch on right of slider
2 participants