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

Add dynamic event display docs #1077

Merged
merged 21 commits into from May 16, 2023
Merged

Conversation

WenzDaniel
Copy link
Collaborator

Before you submit this PR: make sure to put all operations-related information in a wiki-note, a PR should be about code and is publicly accessible

What does the code in this PR do / what does it improve?

Adds dynamic event display tutorial to docs. Before it was quite hidden in the analysis code repository.

@coveralls
Copy link

coveralls commented Sep 7, 2022

Coverage Status

Coverage: 93.44% (-0.01%) from 93.451% when pulling b45d4d6 on add_dynamic_event_display_docs into 0aeb683 on master.

@WenzDaniel
Copy link
Collaborator Author

Added a small fix for the wiki friendly html conversion.

Copy link
Contributor

@JoranAngevaare JoranAngevaare left a comment

Choose a reason for hiding this comment

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

Hi Daniel, nice to add!

  1. Would we need the .md and the notebook both? Otherwise probably adding a link to the .ipynb in the proper repo would probably be enough?
  2. Would you want to include the HTML file into the md file? I'm sure something like that should be possible with something like .. image::
  3. Probably worth changing the headers as below, otherwise they make another section
    image

docs/source/tutorials/EventDisplayInteractive.md Outdated Show resolved Hide resolved
docs/source/tutorials/EventDisplayInteractive.md Outdated Show resolved Hide resolved
@WenzDaniel
Copy link
Collaborator Author

Would we need the .md and the notebook both? Otherwise probably adding a link to the .ipynb in the proper repo would probably be enough?

I think since it is a tool provided along the package it make sense to put it here. In my personal opinion, the other repository has become a bit useless as it is too heavy I stopped using it already a long time ago.

Would you want to include the HTML file into the md file?

Nice idea also though about it, but I did not want to put any unnecessary weight to the repository.

Probably worth changing the headers as below, otherwise they make another section

Ah good catch, I will change it

@JoranAngevaare
Copy link
Contributor

@WenzDaniel should we merge as it is or? I hope that we could somehow embed the html doc into the .md file

@dachengx dachengx self-requested a review May 1, 2023 15:43
@dachengx
Copy link
Collaborator

I think we can only keep the notebooks because users will want to directly run the codes.

@@ -691,6 +691,9 @@
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.8.16"
},
"nbsphinx": {
"execute": "never"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here

"nbsphinx": {
    "execute": "never"
  }

is to disable the execution of the notebook when readthedocs building.

@dachengx dachengx merged commit fbe0c0a into master May 16, 2023
7 checks passed
@dachengx dachengx deleted the add_dynamic_event_display_docs branch May 16, 2023 17:42
@WenzDaniel
Copy link
Collaborator Author

Hi Dacheng thanks for implementing.,

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.

None yet

4 participants