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

Fh/weeeee #561

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fh/weeeee #561

wants to merge 2 commits into from

Conversation

teutoburg
Copy link
Contributor

@teutoburg teutoburg commented Feb 11, 2025

This factors out the wheel-type data structure found in multiple effects. Branch was from 2-3 months ago actually, now found it again and rebased (#500 had to go before). Not sure if I'm quite happy with how this turned out, perhaps after going full dataclass effects I might turn WheelEffect into a subclass of Effect (conceptually more reasonable), but that's for later. The wheels were anyway a major roadblock for the dataclasses, so to keep the PRs at a reasonable size, I decided to make this change separately, as it can (mostly) exist on its own.

Perhaps I'll keep this as a draft until #494 is merged and then merge (or rebase) that in, because #494 also uses wheel-type effects and thus should be included here, but I don't want to make @oczoske's life even harder in that PR...

This is a bit of a hacky WIP, which will improve once Effect is a proper
dataclass and this can inherit from it.
@teutoburg teutoburg added refactor Implementation improvement effects Related to a ScopeSim effect labels Feb 11, 2025
@teutoburg teutoburg self-assigned this Feb 11, 2025
Copy link

codecov bot commented Feb 11, 2025

Codecov Report

Attention: Patch coverage is 90.97222% with 13 lines in your changes missing coverage. Please review.

Project coverage is 77.01%. Comparing base (3cb4fd0) to head (e3b4ca3).

Files with missing lines Patch % Lines
scopesim/effects/effects.py 87.75% 6 Missing ⚠️
scopesim/effects/ter_curves.py 92.59% 4 Missing ⚠️
scopesim/effects/spectral_trace_list.py 89.47% 2 Missing ⚠️
scopesim/effects/apertures.py 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
- Coverage   77.01%   77.01%   -0.01%     
==========================================
  Files          66       66              
  Lines        8219     8217       -2     
==========================================
- Hits         6330     6328       -2     
  Misses       1889     1889              

☔ 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
Labels
effects Related to a ScopeSim effect refactor Implementation improvement
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

1 participant