search

laserkelvin / PySpecTools

Routines for rotational spectroscopy analysis written in Python 3
MIT License
31 stars 5 forks source link

Enhancing AssignmentSession.plot_spectrum() #19

Closed aowen-uwmad closed 4 years ago

aowen-uwmad commented 4 years ago

Our group is used to looking at the spectrum using the intensity, and this is what is initially plotted when using .plot_spectrum() of an AssignmentSession before doing any peak finding. Which makes it particularly jarring when using .plot_spectrum() after peak finding and it "appears" to have completely changed.

There are two main improvements I'd like to make.

  1. Add y-axis label. This will help make it clear what data is being plotted. fig.layout["yaxis"]["title"] = desired_y_label_str

  2. Add option for what type of data is being plotted. In case we'd like to view the spectrum using intensity after peak finding. Could just add a variable call in the function definition and have the user pass the column name of the dataframe they'd like to plot. I'd suggest also adding checks to make sure the SNR column exists before trying to plot that.

laserkelvin commented 4 years ago

The units change after peak finding to units of signal-to-noise: this is simply a conventional because raw intensity units do not necessarily mean anything to others (i.e. a raw volts scale is meaningless without specifying gain, etc). The choice to make it plot in units of SNR is simply a matter for consistency between groups, both in the generated report as well as for publications.

If you change the int_col attribute of an AssignmentSession object, all the plotting and whatnot will be done with that column, regardless of whether you've run peak_find or not. That answers your second point.

Would you like to build the y-axis label change in, and submit a PR?

I have yet to write up a proper guide on how to do this, but if you can take a look at astropy's documentation on good practice for setting up environments for new features/PR's. Please feel free to ask if you're unsure about anything.

aowen-uwmad commented 4 years ago

Changing the attribute is definitely a simpler way of getting the desired result. I will work on adding the y-axis label, following the guide you suggested.

laserkelvin commented 4 years ago

Closing because PR #22 was merged.