Skip to content

Conversation

@altuson
Copy link
Contributor

@altuson altuson commented Jul 9, 2025

In this PR, we update the use of tesswcs to significantly decrease runtime.

We use the function get_pixel_locations() function from tesswcs. Previously, we were calling this at every time. However, in this update, we now call it once per unique queried sector. This significantly decreases the runtime and gives the same result.

@altuson
Copy link
Contributor Author

altuson commented Jul 9, 2025

@jorgemarpa Please could you review and merge this PR at your earliest convenience.

Copy link
Collaborator

@jorgemarpa jorgemarpa left a comment

Choose a reason for hiding this comment

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

This looks good to me. I left a small comment regarding data frame concatenation, but it doesn't change the result.

Comment on lines +164 to +166
df = concat(
[df, result[["time", "Sector", "Camera", "CCD", "Column", "Row"]]]
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure which is faster, if doing a concatenation each time in the for loop vs appending to a list and and then concatenating after the loop.

df = []
for ... in ...:
      ...
      ...
      df.append(result[...])
df = pd.concat(df, axis=0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested this and the two methods take the same time to run, so I suggest we leave the code as is.

Copy link
Collaborator

@jorgemarpa jorgemarpa left a comment

Choose a reason for hiding this comment

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

ready to merge

@jorgemarpa jorgemarpa merged commit 1b8f4a9 into SSDataLab:main Jul 9, 2025
5 checks passed
@altuson altuson deleted the use_sector branch July 9, 2025 20:07
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