Skip to content

Conversation

mkelley
Copy link
Member

@mkelley mkelley commented Nov 11, 2024

New surfaces module, addressing the first step in issue #415.

This PR defines the API for surfaces via the Surface class, and provides support for sunlight scattering off a Lambertian surface:

  • Instances of the Surface class require four methods:
    • absorptance(i)
    • emittance(e, phi)
    • reflectance(i, e, phi)
    • radiance(F_i, i, e, phi)
  • The provided LambertianSurface class defines absorptance, emittance, and reflectance for a surface that uniformly scatters light in all directions.
  • ScatteredLight defines radiance for a surface scattering a light source.
  • The ScatteredSunlight adds scattered_sunlight, which is used to calculate the radiance from a surface scattering sunlight. It is mainly for user convenience.

See the documentation for more.

I'm open to any suggestions, especially if improvements in the names and terminology are needed.

@pep8speaks
Copy link

pep8speaks commented Nov 11, 2024

Hello @mkelley! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 109:63: E203 whitespace before ':'
Line 119:59: E203 whitespace before ':'

Comment last updated at 2025-01-15 13:22:57 UTC

Copy link

Thank you for your contribution to sbpy, an Astropy affiliated package! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the sbpy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related?
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label.
  • Is a milestone added?

Copy link

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 28.85572% with 143 lines in your changes missing coverage. Please review.

Project coverage is 27.55%. Comparing base (5a3f9be) to head (5315e23).

Files with missing lines Patch % Lines
sbpy/surfaces/tests/test_surface.py 0.00% 56 Missing ⚠️
sbpy/surfaces/tests/test_scattered.py 0.00% 28 Missing ⚠️
sbpy/surfaces/tests/test_lambertian.py 0.00% 27 Missing ⚠️
sbpy/surfaces/surface.py 60.00% 14 Missing ⚠️
sbpy/surfaces/scattered.py 58.33% 10 Missing ⚠️
sbpy/surfaces/lambertian.py 61.11% 7 Missing ⚠️
sbpy/units/typing.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #418       +/-   ##
===========================================
- Coverage   83.15%   27.55%   -55.60%     
===========================================
  Files          88       96        +8     
  Lines        8197     8398      +201     
===========================================
- Hits         6816     2314     -4502     
- Misses       1381     6084     +4703     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Successfully merging this pull request may close these issues.

2 participants