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

argument order for filter and threshold changed in find_photons() #25

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

Conversation

ambarb
Copy link

@ambarb ambarb commented Jun 15, 2023

See #24

just switched 250 and 3 in the call to find_photons()

Left output in place as this is how it was rendered before this PR

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ambarb
Copy link
Author

ambarb commented Jun 15, 2023

@stuwilkins or @stuartcampbell i see that you two are the ones associated with this repo. I cannot ask for your explicit review on the right-hand side at the top.

I don't know how to fix the things that failed (or which ones can be safely ignored).

We ran the notebook and code using python3.9. We used scipy environment on jupyterhub so the available centroids is:
image

@ambarb
Copy link
Author

ambarb commented Jun 15, 2023

found possible mistake, need to update

@mrakitin
Copy link
Member

@ambarb, I've created a tag v0.2.0rc3, this change can go into the next release candidate. How ready is your PR?
My plan is to rework the CI for this repository to use GitHub Actions rather than Azure DevOps - that way we can have better CI runs.

@ambarb
Copy link
Author

ambarb commented May 23, 2024

@mrakitin no. there is a mistake here and I need to look back at my slack conversation with the summer student that spend the summer looking at the c-code.

@padraic-shafer FYI given your question on slack

@ambarb
Copy link
Author

ambarb commented May 23, 2024

@mrakitin i fixed the notebook that had the wrong argument

for this branch, the version is:

pycentroids._pycentroids.__version__

'67bb8de+'

do i need to do anything to get the correct tag? The notebook example that is in production shows tag associated with a version release. see the last cell. mine is just a generic commit

as for the results, added None for what I am assuming is the new filter argment finds the same number of of possible events, but some of the finer details make it appear there are some improvements in reducing more outliers.

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