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

docs and formatting for fake_image.py, girsmconf.py, jwst_utils.py #246

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

KostasValeckas
Copy link
Contributor

@KostasValeckas KostasValeckas commented Sep 11, 2024

Updated docs and formatting for fake_image.py, girsmconf.py, jwst_utils.py .

This is a continuation from PR's #229 and #240 as agreed with @gbrammer .

Specifically:

  • added missing / completed incomplete docstrings using AI tools.

  • went through formatting using black.

  • removed or edited some unused parameters

Very little done to fake_image.py .

For girsmconf.py, in the CRDSGrismConf class, I had issues with determining the interplay between the many DISP methods (DISPX, INVDISPX) and all alike, and the model evaluation methods that are being employed within (specially the inverse). I had made a best-attempt, and left some TODO comments on where I got stuck. Many of these methods are repetitive, so please feel free to let me know if/where the docs are incorrect (and if possible what corrections should be done) - I would happily to do another iteration and apply any feedback myself.

In jwst_utils.py, some parameters were never used. These were mostly keyword args to other methods. I have exchanged hardcoded keyword args with the unused parameters where the intention for this was clear. All relevant parts are marked with TODO comments for review.

@KostasValeckas KostasValeckas changed the title docs and formatting for fake_image.py and girsmconf.py docs and formatting for fake_image.py, girsmconf.py, jwst_utils.py Sep 27, 2024
@gbrammer
Copy link
Owner

gbrammer commented Oct 8, 2024

Thanks @KostasValeckas ! Yes, the logic of the grismconf polynomials is very hard to follow. I've updated the docstrings accordingly. Not necessarily such that you can understand exactly what they do but at least the basic description of the inputs and outputs.

@gbrammer gbrammer merged commit 852134f into gbrammer:master Oct 8, 2024
17 checks passed
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