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

Advanced Composite Model Fitting notebook #74

Merged
merged 3 commits into from
Dec 2, 2019

Conversation

hcferguson
Copy link
Contributor

Draft of notebook from @ibusko , annotated and partially revised by @hcferguson to use some of the SpecUtils machinery.

@eteq
Copy link
Collaborator

eteq commented Nov 27, 2019

I pushed up a few commits to fix the dependencies, and I think the build is now working. @hcferguson, can you confirm that https://26-126393017-gh.circle-artifacts.com/0/root/repo/jdat_notebooks/composite_model_fitting/specfit_demo_3.html looks the way you think it should look for this notebook?

@eteq
Copy link
Collaborator

eteq commented Nov 27, 2019

Oh and one clarifying question for @hcferguson: in the requirements file you specified == for your versions. Is that intended, or do you mean >=? I.e., you were specifying the version you are running on, but not saying that you know for sure it doesn't work on a more recent version.

(This also reveals that a bit more detail on what goes in the requirements file is called for in the README...)

@hcferguson
Copy link
Contributor Author

The html rendering looks good to me.

Requirements >= should be fine. I was just mindlessly following the example.

@eteq
Copy link
Collaborator

eteq commented Nov 27, 2019

Requirements >= should be fine. I was just mindlessly following the example.

Oh good point! I should change the example to be >=...

eteq added a commit that referenced this pull request Nov 27, 2019
Copy link
Collaborator

@eteq eteq left a comment

Choose a reason for hiding this comment

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

This LGTM now - thanks @hcferguson !

@eteq eteq merged commit 84134e2 into spacetelescope:master Dec 2, 2019
camipacifici pushed a commit to camipacifici/dat_pyinthesky that referenced this pull request Dec 12, 2019
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.

2 participants