Skip to content

Corgisim dataset#732

Open
helloWorlds0 wants to merge 4 commits intodevelopfrom
corgisim_dataset
Open

Corgisim dataset#732
helloWorlds0 wants to merge 4 commits intodevelopfrom
corgisim_dataset

Conversation

@helloWorlds0
Copy link

@helloWorlds0 helloWorlds0 commented Jan 29, 2026

Describe your changes

Type of change

added some option so that WFOV band 4 data can be processed

  • New feature (non-breaking change which adds functionality)

Reference any relevant issues (don't forget the #)

Checklist before requesting a review

  • I have verified that all unit tests pass in a clean environment and added new unit tests, as appropriate
  • I have checked that I am merging into the right branch
  • I have checked the output of the latest Github Actions run associated with this PR and confirmed running pytest did not produce any warnings
  • I have checked if my code modifies an existing step function or modifies the data format docs. If it does, I've added my PR to the list maintained here.

@helloWorlds0 helloWorlds0 marked this pull request as ready for review January 29, 2026 18:04
Copy link
Contributor

@semaphoreP semaphoreP left a comment

Choose a reason for hiding this comment

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

Thanks for finding these fixes. I have one comment below and one suggestion for a unit test:

We could copy test_klipthrupt.test_psfsub_withklipandctmeas_adi() and do it for Band4 WFOV. For the unit tests, we don't care what the data values look like, so we just want to adjust the unit test so the image shape is correct, and modify the header keywords (e.g., after the function runs create_psfsub_dataset(), manually modify some of the header values). Hopefully this is a relatively straightforward adaptation of an existing unit test.

corgidrp/data.py Outdated
# Set filter wavelengths
self.wave_hlc = {'1F': 575e-9} # meters

self.wave_spc = {'4F': 825e-9} # meters
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to make these separate dictionaries, as the filter wavelengths won't change with masks. Maybe let's just call it self.filter_wavs rather than self.wave_hlc and put both filters in the same dictionary.

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