-
Notifications
You must be signed in to change notification settings - Fork 14
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
[MRG] Start organizing examples properly #30
Conversation
examples/plot_power.py
Outdated
from params_power import main_path, data_path | ||
from params_power import subject_ids, sessions | ||
from params_power import power_analysis_name | ||
from params_power import fmin, fmax, power_method, is_epoched, freq_bands |
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.
@annapasca freq_bands
was not imported in your example online. You can't detect such issues unless you do continuous integration ...
0b5a9b1
to
001b440
Compare
Hello @annapasca can you do the following: $ git fetch upstream pull/30/head:fix_examples
$ git checkout fix_examples
$ cd doc/
$ make html Are you able to make it work? For me, it doesn't work if I download the files that you shared on dropbox. %run examples/plot_01_preprocessing.py in Also, if you do: $ make install you should be able to push to http://neuropycon.github.io/ephypype and update the docs in one command. I also implemented automatic data fetcher. It's okay for now but the data is really big -- 3.6 GB!!! We should use a smaller dataset later. But one step at a time ... |
f832d7e
to
97171e8
Compare
Hi @jasmainak !
but I got the error
|
If you do: >>> from ephypype.datasets import fetch_omega_dataset does it work? It should work ... do you do an editable install -- that is |
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
- Coverage 29.19% 27.98% -1.22%
==========================================
Files 19 19
Lines 1459 1458 -1
==========================================
- Hits 426 408 -18
- Misses 1033 1050 +17
Continue to review full report at Codecov.
|
48ccddb
to
52af9d0
Compare
@annapasca I managed to get the preprocessing and the power examples to "almost" work with sphinx gallery. Both of them get stuck -- and I don't know why. The issue of the missing $ git checkout fix_examples
$ git pull upstream pull/30/head
$ cd doc/
$ make html Also, would you have a moment in the coming days to discuss on skype the API and simplifying the example? Maybe you can also help me understand why my examples get stuck ... |
@EtienneCmb would you have some time to do the same as @annapasca ? To check that this PR works ... |
Hi @jasmainak! I tried and I'm working in the I could install anaconda2 to see if also for me using your env I have the same problem. What do you think? |
About the Skype, is it ok for you tomorrow 10 a.m. Mtl time? |
Yep that works! Today?
On Wed 19 Sep 2018 at 05:52, Annalisa Pascarella ***@***.***> wrote:
About the Skype, is it ok for you 10 a.m. Mtl time?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APHiopWaVsC8TXvL7nU-rBRnVHGjEkpdks5uchPPgaJpZM4WqK9F>
.
--
Sent from my iPhone
|
No I think I’m using MNE env now and it still doesn’t work ...
On Wed 19 Sep 2018 at 05:51, Annalisa Pascarella ***@***.***> wrote:
Hi @jasmainak <https://github.com/jasmainak>!
I tried and make html works fine! The 2 workflows run properly.
I'm working in the mne env. Since I use anaconda3, your env is not fine
for me.
I could install anaconda2 to see if also for me using your env I have the
same problem.
How do you think?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APHiolaPn6-BMATlK8PwQkDOQapJhNCxks5uchOXgaJpZM4WqK9F>
.
--
Sent from my iPhone
|
Tmw? :) |
okay that's fine :) I saw your message from email first, so didn't see the edited message. So, we speak in 24 hours from now ... |
@jasmainak I changed 2 files to do new test on only 1 min of raw data (ICA seems to work). I only changed in |
You only changed the data right? It's on the same dropbox link so you don't need to push on this PR ... it should fetch the data automatically. But if you want to know how to do it, you would do: $ git remote add jasmainak https://github.com/jasmainak/ephypype
$ git remote push jasmainak fix_examples |
|
Show me the full log? |
You need to scroll up a bit to see the actual traceback ... |
|
It should be specified at the top of your examples that some data are needed. I run the examples and it downloaded 675,9 Mo of data files... |
okay sure. I'm refactoring the example a bit so you will be able to soon see the fetch data command. |
b3cf79b
to
f9f4da4
Compare
alright @EtienneCmb preprocessing example made more pedagogic. It could be made better, but it's a start ... |
24b6fe0
to
2687152
Compare
c5197ad
to
3af52df
Compare
@@ -116,7 +74,6 @@ Install dependencies with conda | |||
.. code-block:: bash | |||
|
|||
conda install pandas | |||
conda install ipywidgets |
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.
@annapasca @dmalt I removed this. I think we discussed on hangouts that it is not used anywhere, right?
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.
yes, for me it's ok!
If someone wants to inspect the ICA by notebook, he'll do it by himself.
What do u think @dmalt ?
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.
oh sorry I didn't know that. Do you have the link for the notebook? So I can see what it is doing ...
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.
|
||
see |MNE install| for more information. | ||
|
||
|
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.
@annapasca I also removed installation instruction for other packages. I think it's not so relevant here. We should simplify things to not bring people with MNE installation problems to neuropycon
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.
ok! you are right! :)
2641ff6
to
93ef662
Compare
import os.path as op | ||
|
||
path = op.join(op.dirname(__file__), '../examples/') | ||
sys.path.insert(0, path) |
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.
@annapasca we should remove L201
and L202
after you reformat the power example ...
61a28a6
to
4561c71
Compare
4561c71
to
09f65ba
Compare
@annapasca I made CircleCI work now. I think this is good to go :) Can you merge the pull request and work on top of it to improve the other examples? Waiting for your next pull request! See the output here. Couple of outstanding issues to be fixed later:
@dmalt I cleaned up the API page. However, as you mentioned today that there are other nodes which should be in public API, can you a make a pull request to add those? I am going to focus on my other projects for the next couple of weeks. But I'll still look at the updates and pull requests. I hope we now have a good start and can keep the momentum going. Cheers! |
Hi @jasmainak thanks a lot for this big review! I'm going to merge all and next week I'll work on both example and unit test. Since I'd like to learn how to be not too messy :D the right way after merge the PR is:
I guess I will not be promoted :D :D :D |
You should do: $ git fetch upstream master:master
$ git checkout master
$ git branch my_feature_branch
$ git checkout my_feature_branch |
Good luck :) |
Grazieee!!! :) I have to study more ;) |
Organizing the examples properly so that they can be built using sphinx gallery ...