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

Harmonize the column name for mag errors #374

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

Conversation

joezuntz
Copy link
Collaborator

No description provided.

Copy link
Contributor

@empEvil empEvil left a comment

Choose a reason for hiding this comment

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

Looks good, I see no issues with this. I am a little confused though, It only looks like we are loading in the mag_err in ingest part of the pipeline, but are we actually using it somewhere? if no, why are we loading it? if yes, does the name change need to be there as well?

Copy link
Collaborator

@elvinpoole elvinpoole left a comment

Choose a reason for hiding this comment

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

looks good to me

@elvinpoole
Copy link
Collaborator

Looks good, I see no issues with this. I am a little confused though, It only looks like we are loading in the mag_err in ingest part of the pipeline, but are we actually using it somewhere? if no, why are we loading it? if yes, does the name change need to be there as well?

I think if we want run the redmagic selection within TXPipe (which was discussed at some point) we will need these mag errors. so i think good to keep them (have use the same notation)

@empEvil
Copy link
Contributor

empEvil commented Nov 26, 2024

that is a fair point, it is good for future proofing. I was just curious

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