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

Added a pydantic-validated Camera class and test. #163

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

JuanCab
Copy link
Contributor

@JuanCab JuanCab commented Aug 22, 2023

I have added a pydantic (v1) validated Camera class. I also had to modify previous units checks (we now through ValueError instead of TypeError) and added a new check to see that any 'count' (e.g. count or electron or whatever) is acceptable as long as the units of read_noise, gain, and dark_current are consistent.

I also confirmed we can json serialize the Camera object, so we can dump its value to disk and should be able to deserialize it easily enough.

@JuanCab JuanCab added the refactor Summer 2023 project to rewrite stellarphot label Aug 22, 2023
@JuanCab JuanCab requested a review from mwcraig August 22, 2023 19:46
Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

Despite the extensive comments I think this is really close to ready. Sorry it took a while -- I needed some uninterrupted time so I could try things locally since I'm not that familiar with pydantic or Quantity.

In addition to the line suggestions, I have one other:

Add an "image_unit" attribute to Camera so that the user has the option of using whatever image unit that want (ADU, count, something that is already gain correct so that it is in electrons, say)

What do you think?

stellarphot/core.py Show resolved Hide resolved
stellarphot/core.py Show resolved Hide resolved
stellarphot/core.py Outdated Show resolved Hide resolved
stellarphot/core.py Outdated Show resolved Hide resolved
stellarphot/core.py Outdated Show resolved Hide resolved
stellarphot/tests/test_core.py Outdated Show resolved Hide resolved
stellarphot/tests/test_core.py Outdated Show resolved Hide resolved
stellarphot/tests/test_core.py Outdated Show resolved Hide resolved
stellarphot/tests/test_core.py Outdated Show resolved Hide resolved
stellarphot/core.py Show resolved Hide resolved
Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

Thanks!

@mwcraig mwcraig merged commit c40a522 into feder-observatory:main Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Summer 2023 project to rewrite stellarphot
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants