Skip to content

Adding moving stack funtionality#12

Open
johnwdailey wants to merge 6 commits into
Small-Bodies-Node:mainfrom
johnwdailey:main
Open

Adding moving stack funtionality#12
johnwdailey wants to merge 6 commits into
Small-Bodies-Node:mainfrom
johnwdailey:main

Conversation

@johnwdailey

Copy link
Copy Markdown
Collaborator

I believe this should add moving stack functionality to the catch-analysis-tools repo. But this is my first time using this functionality of github, so it's entirely possible that there will be some kind of error.

Comment thread catch_analysis_tools/movingstack.py Outdated
"""
Create an undistorted WCS object based on a given reference position and image size.

Parameters:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're using the numpydoc format for documentation. So, something like this:


    """
    Create an undistorted WCS object based on a given reference position and image size.


    Parameters
    ----------
    ra : float
        Right Ascension (RA) of the reference position in degrees.

    dec : float
        Declination (Dec) of the reference position in degrees.

    size_x : int
        Number of pixels along the x-axis (image width).

    size_y : int
        Number of pixels along the y-axis (image height).

    pixel_scale : float
        Pixel scale in arcseconds per pixel (default is 0.4"/pixel).


    Returns
    -------
    wcs : astropy.wcs.WCS
        The new world coordinate system object.

    """

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I believe this has been updated accordingly, but there were a couple of places where I wasn't 100% clear on the desired style

Comment thread catch_analysis_tools/movingstack.py Outdated
try:
return header["MAGZPT"]
except:
return defaultzeropoint

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about raising a warning if we get to this point?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can if it's something you would like to do. As I recall from mid development testing there are seemingly entire surveys for which this will happen.

Comment thread catch_analysis_tools/movingstack.py Outdated
Comment thread catch_analysis_tools/movingstack.py Outdated
"""
return arg.strip('\'"')

parser = argparse.ArgumentParser()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're going to have the API call the code via a funcion. Can you repackage the rest of this into a main stack() function or similar? I don't mind the CLI intferface, but I also don't think it is needed. I'll leave that decision up to you. If you want to keep it, the general approach would be:

if __main__ == "__main__":
    argument parsing...
    stack(args...)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is functionality I haven't used before. I would be happy to implement it if you could point me to an example of it being used.

I would prefer to keep the command line because I think it is helpful for debugging/testing.

@johnwdailey johnwdailey self-assigned this May 5, 2025
@johnwdailey

Copy link
Copy Markdown
Collaborator Author

I have pushed updates to the code for most of your comments. The part about the arg parsing and the stack are not yet done because I would benefit from being pointed to an example of what you're describing.

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