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

solve issue 236 #253

Merged
merged 3 commits into from
Oct 29, 2024
Merged

solve issue 236 #253

merged 3 commits into from
Oct 29, 2024

Conversation

GallegoSav
Copy link
Contributor

No description provided.

@GallegoSav GallegoSav added the fix issue pull request for solving issue label Oct 22, 2024
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.66%. Comparing base (b9ac312) to head (86e508d).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
cosipy/data_io/UnBinnedData.py 75.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
cosipy/data_io/UnBinnedData.py 94.87% <75.00%> (-0.34%) ⬇️

@ckarwin ckarwin self-assigned this Oct 22, 2024
Copy link
Contributor

@ckarwin ckarwin left a comment

Choose a reason for hiding this comment

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

@GallegoSav Thanks for taking care of this. The changes look good, and the logic makes sense. I added a few minor comments to clean up the code before merging. I'll check again when you make the updates.

@@ -185,6 +185,16 @@ def read_tra(self, output_name=None, run_test=False, use_ori=False,
if len(this_line) == 0:
continue

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove empty white space (lines 188 and 189 )


# Event type:
if this_line[0] == "ET":
#Check we are looking CO evt
Copy link
Contributor

Choose a reason for hiding this comment

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

#Check we are looking CO evt --> # Check that we are looking at CO events

# Event type:
if this_line[0] == "ET":
#Check we are looking CO evt
if this_line[1] == "CO" :
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove space before colon.

#Check we are looking CO evt
if this_line[1] == "CO" :
et.append(this_line[1])
else :
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove space before colon.

# Event type:
if this_line[0] == "ET":
et.append(this_line[1])


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty space (line 232)

Copy link
Contributor

@ckarwin ckarwin 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, thanks @GallegoSav.

@ckarwin ckarwin merged commit 0ea3e15 into cositools:develop Oct 29, 2024
2 checks passed
@GallegoSav GallegoSav deleted the develop_issue236 branch January 10, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix issue pull request for solving issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants