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

lt_xspec: Auto z and more #417

Merged
merged 3 commits into from
Oct 17, 2017
Merged

lt_xspec: Auto z and more #417

merged 3 commits into from
Oct 17, 2017

Conversation

profxj
Copy link
Contributor

@profxj profxj commented Oct 5, 2017

Loads linelist and redshift if
.z and .type='galaxy' in spectrum object.

@profxj profxj requested a review from jnburchett October 5, 2017 13:40
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 71.692% when pulling d7e134f on auto_z_and_more into 9ff0b74 on master.

Copy link
Collaborator

@jnburchett jnburchett left a comment

Choose a reason for hiding this comment

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

A couple things that may or may not be bugs, one regarding multiple spectra. If you don't intend to address this behavior on the current PR, it's fine. The second bug (linelists) actually negates the added feature somewhat, so we should try and get it straightened out.

# Reset redshift from spec
if zsys is None:
if hasattr(self.spec_widg.spec, 'z'):
self.pltline_widg.setz(str(self.spec_widg.spec.z[self.spec_widg.select]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried this feature out with spectra from a specdb.spectra_from_coord() query, and the first spectrum has its redshift set, but the redshift does not change when I select a different spectrum. Can the multiSpecWidget talk back to the main gui to change redshift when selecting different spectra?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed; great catch

self.pltline_widg.llist['Plot'] = True
idx = self.pltline_widg.lists.index(self.pltline_widg.llist['List'])
self.pltline_widg.llist_widget.setCurrentRow(idx)
#
Copy link
Collaborator

Choose a reason for hiding this comment

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

Loading up the GUI automatically selects the correct linelist for 'Galaxy', but the lines do not immediately show. Instead, I have a click on a different linelist in this widget then then back to Galaxy. At that point, I see the galaxy lines plotted. This is quite possibly a Qt 'idiosyncracy' and perhaps works properly on other platforms. Are the lines marked when you open up the GUI on your machine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it is an idiosyncracy..

@@ -86,7 +98,8 @@ def __init__(self, ispec, parent=None, zsys=None, norm=None, exten=None,

# Extras
extras = QWidget()
extras.setMaximumWidth(130*self.scale)
extras.setMinimumWidth(180*self.scale)
extras.setMaximumWidth(280*self.scale)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@@ -7,3 +7,4 @@ v1.1
Adding in X-ray lines to EUV
v1.2
Adding some extra forbidden lines for Galaxy (NT Jun 28, 2017)
Turned off distracting higher order Balmer lines in Galaxy
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once again, thanks! Much cleaner for higher-z galaxies.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 71.709% when pulling 02f8006 on auto_z_and_more into 9ff0b74 on master.

@profxj
Copy link
Contributor Author

profxj commented Oct 17, 2017

Is this ready, @jnburchett ?

@jnburchett jnburchett merged commit 44f0e74 into master Oct 17, 2017
@profxj profxj deleted the auto_z_and_more branch November 4, 2017 12:38
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.

None yet

3 participants