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

Add summary of capabilities/user stories to docs index page #149

Merged
merged 4 commits into from
Nov 29, 2022

Conversation

rosteen
Copy link
Contributor

@rosteen rosteen commented Nov 22, 2022

As it says in the title - I hope that this is something close to what you were imagining for the "user stories" on the docs landing page, to give people an idea of what they might use specreduce for. If I'm off the mark let me know.

@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #149 (82675f6) into main (2a63e7d) will increase coverage by 2.86%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   72.60%   75.46%   +2.86%     
==========================================
  Files           9        9              
  Lines         646      693      +47     
==========================================
+ Hits          469      523      +54     
+ Misses        177      170       -7     
Impacted Files Coverage Δ
specreduce/background.py 88.29% <0.00%> (-0.99%) ⬇️
specreduce/tracing.py 93.54% <0.00%> (+0.78%) ⬆️
specreduce/extract.py 95.30% <0.00%> (+6.98%) ⬆️
specreduce/core.py 74.35% <0.00%> (+27.30%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@ojustino ojustino left a comment

Choose a reason for hiding this comment

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

Hi @rosteen, thanks for the pull request. I like the content and think it would be good to rearrange it. My suggestions:

  • Leave the first sentence as is ("The specreduce package aims...").
  • Combine the beginning of the second sentence (up to "...basic spectroscopic reduction") and your first sentence ("Specreduce is currently useful...") into something like "Specreduce is currently most useful for the following steps in basic spectroscopic reduction:"
  • Leave your numbered list as is.
  • Paste the rest of the original second sentence after the list – I think it's better to list what we can't do and where else to go only after what we've given details on what we can do.

@rosteen
Copy link
Contributor Author

rosteen commented Nov 28, 2022

@ojustino I made some edits, does it sound better to you now?

Copy link
Contributor

@ojustino ojustino left a comment

Choose a reason for hiding this comment

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

This looks comprehensive to me. I have some assorted suggestions and am ready to approve once you've made decisions on them.

docs/index.rst Outdated
basic spectroscopic reduction, currently encompassing the following three tasks:

#. Determining the trace of a spectrum dispersed in a 2D image, either by setting a flat
trace or fitting a spline, polynomial, or other model to the positions of the dispersed
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also add that users can set a custom trace so ArrayTrace isn't left out.

rosteen and others added 2 commits November 29, 2022 15:00
Co-authored-by: ojustino <ojustino@users.noreply.github.com>
@rosteen
Copy link
Contributor Author

rosteen commented Nov 29, 2022

@ojustino I put in all your suggestions, thanks!

Copy link
Contributor

@ojustino ojustino left a comment

Choose a reason for hiding this comment

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

Thanks again, @rosteen.

@ojustino ojustino merged commit da21b5e into astropy:main Nov 29, 2022
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