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

Change default data layer opacity to 1 #571

Merged
merged 2 commits into from
Apr 27, 2021

Conversation

astrofrog
Copy link
Collaborator

In glue, the default opacity for data layers is 0.8 - while it might make sense to change the default there, for now the quickest and most past- and future-proof fix is to simply set the default for jdaviz here.

Fixes #535
Fixes #537

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.

Wow, I didn't know how much this bothered me until I looked at Cubeviz with this change...it looks so much better now in the initial display at max opacity. I confirmed that overplotting a dataset hides the other with opacity set to the max now.

I left two suggestions, settings is very generic and I thought importing as glue_settings would make it more clear what's being set later in the code.

jdaviz/app.py Outdated Show resolved Hide resolved
jdaviz/app.py Outdated Show resolved Hide resolved
@pllim pllim added imviz bug Something isn't working labels Apr 27, 2021
@pllim pllim added this to the Imviz 1.0 milestone Apr 27, 2021
Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM and I also agree with review comments from @rosteen .

@astrofrog
Copy link
Collaborator Author

@rosteen - I agree and have accepted your suggestions!

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.

Approved, I'll merge after the tests finish running.

@rosteen rosteen merged commit f78de78 into spacetelescope:main Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working imviz
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default opacity in Imviz should be 1 BUG: Imviz opacity
3 participants