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

plotspectra outfile and with_other_model opts #99

Merged
merged 2 commits into from
May 1, 2024
Merged

Conversation

sbailey
Copy link
Collaborator

@sbailey sbailey commented Apr 17, 2024

This PR adds two backwards compatible options to plotspectra:

  • outfile: specify the name of the output file instead of the default html_dir/title.html (I want to set the title in the plot to be more verbose than I want for the name of the file)
  • with_other_model=True; set to False to turn off the other/default model loading

I'm not entirely sure I got the othermodel widget wrangling correct for all cases of the default templates vs. 2nd best fits vs. nothing, so please check how that was intended to work and that I didn't break anything.

@sbailey sbailey requested a review from armengau April 17, 2024 23:38
@coveralls
Copy link

coveralls commented Apr 17, 2024

Coverage Status

coverage: 2.01% (-0.004%) from 2.014%
when pulling 7ba0cd6 on plot_options
into a697b41 on main.

Copy link
Member

@weaverba137 weaverba137 left a comment

Choose a reason for hiding this comment

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

This seems straightforward to me, OK to merge.

@weaverba137
Copy link
Member

Are there any issues that this PR closes?

@armengau
Copy link
Collaborator

@sbailey I changed the criterium to call viewer_widgets.add_model_select:
if (num_approx_fits is not None and num_approx_fits>0) or with_full_2ndfit
becomes
if with_other_model

Otherwise it would generate buggy pages in the case one plots (spectra only + standard templates)
(a situation which is not theoretical, eg blind VI of secondary targets)

@armengau
Copy link
Collaborator

I ran the tests (examples_prospect_pages.sh), and checked some of the html pages.
Also added a test with with_other_model=False
=> ok for me.

@sbailey sbailey merged commit b3bf989 into main May 1, 2024
10 checks passed
@sbailey sbailey deleted the plot_options branch May 1, 2024 05:34
@sbailey
Copy link
Collaborator Author

sbailey commented May 1, 2024

Thanks for the review and the fixes. This PR doesn't address any pre-existing tickets; it was a spontaneous contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants