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

Added documentation for the polling detection module #601

Merged
merged 33 commits into from
Apr 11, 2023

Conversation

dan-stats-1
Copy link
Contributor

I've added a jupyter notebook documenting the polling detection module which is stored in the docs/notebooks directory.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ianhelle
Copy link
Contributor

Hey @danielyates2
Looks like a great submission. A couple of questions about this PR.

  1. Does it supercede pr Added polling detection module  #596 ?
  2. Also there seems to be a junit.xml file in the commit. Can you remove that?
    If you can confirm 1 we can close the other PR and just review this one.

@dan-stats-1
Copy link
Contributor Author

Hey @danielyates2 Looks like a great submission. A couple of questions about this PR.

1. Does it supercede pr [Added polling detection module  #596](https://github.com/microsoft/msticpy/pull/596) ?

2. Also there seems to be a junit.xml file in the commit. Can you remove that?
   If you can confirm 1 we can close the other PR and just review this one.

Hey @ianhelle

Thanks!

  1. Yes, it does supercede pr Added polling detection module #596, sorry for the duplication
  2. I've removed the junit xml file and pushed so hopefully everything should be sorted

If there is anything else you need, please let me know

Cheers
Dan

Copy link
Contributor

@ianhelle ianhelle left a comment

Choose a reason for hiding this comment

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

This is very cool but I think we could make it more accessible for people with a simpler interface that does a lot of the transforms for them and they can just supply a DF.
We might also (perhaps v2) be able to have a simple visualization (I really like the 24hr clocks)
or something that hilighted the edges/groups with low p_values.

Awesome stuff!

msticpy/analysis/polling_detection.py Outdated Show resolved Hide resolved
msticpy/analysis/polling_detection.py Outdated Show resolved Hide resolved
@ianhelle
Copy link
Contributor

/azpipeline run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@ianhelle ianhelle left a comment

Choose a reason for hiding this comment

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

I've got a couple of requests in the comments that would make it more accessible/usable by users.

@dan-stats-1
Copy link
Contributor Author

@danielyates2 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

Contributor License Agreement

@microsoft-github-policy-service agree

@ianhelle
Copy link
Contributor

/azpipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ianhelle ianhelle merged commit 703459c into microsoft:main Apr 11, 2023
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