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

Feature/make slit illum interactive #330

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

olesjas
Copy link

@olesjas olesjas commented Feb 25, 2022

list of changes:

  • Added several parameters to the makeSlitIllum primitive, mostly following IRAF task example. E.g. the possibility to supply bins as list of regions, the option to specify sample regions along the spatial axis, interactive reduction.
  • Replaced the final step of creating slit function from fitting a 2D model to 1D row interpolation.
  • Two interactive interfaces were created for makeSlitIllum, for dispersion bin inspection and editing, and for dispersion bin fitting. For the first interface I had to make a specialized visualizer to be able to use regions interface without having to deal with all the fitting-related parts (bineditor.py).
  • By default, the extrapolated parts of slit function are being set to a constant value equal to that of the last interpolation point (as it is done in IRAF), but it can be set to use extrapolation.

Olesja Smirnova and others added 20 commits January 4, 2022 09:51
…itive. Used the code from normalizeFlat as an example to make bin fitting interactive.
…m vizualizer (bineditor.py) by subclassing PrimitiveVizualizer.
… below buttons for makeSlitIllum tabs to have room underneath
@olesjas olesjas closed this Mar 24, 2022
@olesjas olesjas reopened this Mar 28, 2022
@olesjas olesjas closed this Mar 28, 2022
@olesjas olesjas reopened this Mar 28, 2022
Copy link
Contributor

@olyoberdorf olyoberdorf left a comment

Choose a reason for hiding this comment

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

Looks great - thanks!

@chris-simpson
Copy link
Contributor

I've pushed a number of commits to feature/makeSlitIllum_reviewing -- I created a new branch because I changed the way the weights were calculated when collapsing the bins and thought this might cause regression tests to fail. It's also recently had master merged into it. Some are small changes to the calculations, some are simply stylistic to shorten the code (which I think is always good, as long as it doesn't significantly affect the readability).

I would also favour removing the logging statements as I don't think they really add anything. The level of detail in the logs varies a lot from primitive to primitive but generally I would suggest a logging statement for the user whenever something slow is about to happen, or when a non-standard path is taken. Logging should also be used (perhaps at the debug level) when numbers are read from files (to make sure the correct number is being read) or when an if statement causes a decision to be made, to aid with debugging.

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.

3 participants