-
Notifications
You must be signed in to change notification settings - Fork 118
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 YASA Flaskified details to README and FAQ #198
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bartromb ! Looks good, I just had one suggestion on the README.rst file.
README.rst
Outdated
Related Projects | ||
~~~~~~~~~~~~~~~~~ | ||
|
||
Looking for a web-based solution to utilize YASA? Check out **YASA Flaskified**: a Flask-based application designed to process and analyze EEG data via an intuitive web interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer some kind of bullet list here, e.g.:
- `YASA Flaskified <https://github.com/bartromb/YASAFlaskified>`_: a Flask-based application designed to process and analyze EEG data via an intuitive web interface. YASA Flaskified also supports straightforward deployment on your own physical or virtual server using a preconfigured deployment script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, a bulleted list is preferred. And actually I think that the bullet could be placed under the "Development" subheading. A section titled "Related Projects" might be overkill for just this, unless we plan to add other sleep analysis software there as well. If left as-is, I would probably add some other packages to the list later.
<div id="related_projects" class="panel-collapse collapse"> | ||
<div class="panel-body"> | ||
|
||
**YASA Flaskified** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'd love to hear from @remrama's before final approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The docs just got changed to restructured text (instead of HTML). The formatting is much simpler, but I'm not sure how to handle this merge (since Overhaul docs with PyData Sphinx Theme #194 is already merged into
master
). If anyone needs help with this, let me know. - Did an extra
pip install --upgrade yasa
get added to Line 222? If so, that should be removed. - I think the dropdown heading could be more informative than Are there any related projects? What about Can I use YASA online? or Can YASA be deployed on a remote server? If that makes sense...
docs/faq.rst
Outdated
- **Deployment Options:** Users can deploy YASA Flaskified on their own physical or virtual server using a preconfigured deployment script. This allows for private, secure, and scalable use of the application. | ||
- **Integration with YASA:** The application is built around the YASA library, ensuring full compatibility and access to its core features, including sleep staging, event detection, and more. | ||
|
||
**Where to find it?** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- In general this section could have lots of text reduced. Try to avoid headings/questions within FAQ headings. For example, instead of "Where to find it?" and then answering, just say "YASA Flaskified can be found at [insert link]", or even better, just make a statement about YASA Flaskified with a hyperlink.
- But we don't need a long description -- people should be going to the YASA Flaskified site for all the details. This helps long-term maintenance, so if something changes about YASA Flaskified, we don't have to update things here as well.
- I would suggest to be clearer that this is requires deployment of a server. I know it is said here, but I would lead with that in the first sentence somehow. While this tool might be user-friendly for developers, I think many casual users might be misled into thinking they can just drag their files into the tool and run analyses. Including words like "server" and "deploy" will be enough to drop the hint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Related Projects
YASA Flaskified is a web-based tool built on YASA that allows EEG analysis through a browser interface. It requires deployment on a server, which can be done using the preconfigured scripts available in the repository. For more details, visit the **[YASA Flaskified GitHub repository](https://github.com/bartromb/YASAFlaskified)**"
Hi @raphaelvallat, I’ve made the requested changes to the pull request:
You can view the updated changes here. Let me know if there’s anything else you’d like me to adjust! Best regards, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bartromb, thanks for making another adjustment. I like it! I think this should be merged after one last clarification:
I would only switch the markdown hyperlinks back to restructured text format. They were right in the first commit (using backticks `this <format>`_
), but they are markdown in the latest version ([this](format)
). As far as I know, the markdown won't render in the .rst
file, but let me know if I'm wrong about this.
You are completely right. I’ll switch them back to the proper reStructuredText format as they were in the first commit. Thank you for catching that! I’ll make this adjustment right away. Best regards, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #198 +/- ##
=======================================
Coverage 77.24% 77.24%
=======================================
Files 13 13
Lines 2659 2659
Branches 324 324
=======================================
Hits 2054 2054
Misses 568 568
Partials 37 37 ☔ View full report in Codecov by Sentry. |
This pull request adds information about YASA Flaskified, a web-based application built using the YASA library, to the project's documentation:
These changes aim to introduce users to YASA Flaskified as an extension of YASA for those who prefer a web-based interface or need deployment options for their own servers.