Skip to content

Fix tutorials #1036

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

Closed
wants to merge 71 commits into from
Closed

Fix tutorials #1036

wants to merge 71 commits into from

Conversation

duboism
Copy link
Contributor

@duboism duboism commented Jan 30, 2015

I have started to correct some tutorials. Don't hesitate to suggest other improvements.

desc='Name of person to say hello to')


class HelloWorldOutputSpec(MatlabInputSpec):
Copy link
Member

Choose a reason for hiding this comment

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

perhaps this should just be TraitedSpec like example 1. does this also answer your question on neurostars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try that ASAP (and it answers my question on neurostars).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it OK to put the python source code in a separate file (for example matlab_example2.py) and include it with literalinclude? AFAI can see, the ouput is the same. The main advantage is that we can use a python editor to ensure PEP8-ness and test the code. Moreover we could offer the option to download it as it is done in the smri_ants_build_template example.

Copy link
Member

Choose a reason for hiding this comment

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

that's sounds good. all of those are done using py files that get converted from python to rst.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Therefore I will move the 2 examples here to files.

@duboism
Copy link
Contributor Author

duboism commented Feb 2, 2015

I have meet some problems related to sphinx. Could you have a look at this question?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 70.36% when pulling 717555e on duboism:fix_tutorials into 6601b00 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 70.36% when pulling 54fefed on duboism:fix_tutorials into 6601b00 on nipy:master.

@duboism
Copy link
Contributor Author

duboism commented Feb 6, 2015

I have met other sphinx related errors. See the message here.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 70.36% when pulling 868b248 on duboism:fix_tutorials into 6601b00 on nipy:master.

@duboism
Copy link
Contributor Author

duboism commented Sep 14, 2015

The rebase should be finished. I hope I haven't destroyed any other work in correcting the many conflicts. Don't hesitate to warn me in this case (but I can work on nipype only during my spare time).

@@ -42,8 +42,9 @@ deploy:
provider: pypi
user: satra
password:
secure: ZaN6Y4hfybPMi1rW3Qe03irCe/3GU4J4BJsjG+tClVN5Fjc3FCJaYU0k4wNd9q2kMjuRDv1efA9BN/5QasJS32oqs6wHvxzIWA18ucel4BBnAAkuviC1vqi/8Hk/DGB6e1gnmZ1Nv58zVLfn0MwlOKmsGDtvGQWc+HbaV2fS5NA=
secure: OCO0FXb4f+pH4Uw7zWCIRp3qOJ1t7rhky4K8MjNU8tyVCJgd6O/Bv8GJgceS0LktPodlAAjB8SxAhTORPAQZ1D/44PJYy3NQIisvej1zjLpaA9TEGfl6W7MqhDpRyMHW+cnSi/n84SAmdr+Z4vOxScDHdwr13EPmGyOIlHMAGnE=
Copy link
Member

Choose a reason for hiding this comment

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

was this an accidental change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember having touched that file. So probably an accident. I will reverted this file.

Copy link
Member

Choose a reason for hiding this comment

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

this is not comparing properly with master - those changes are already in there. @duboism - could you please close this pull request and send a new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@satra Are you talking about the file .travis.yml or about all the modifications?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum, I have just cloned master in a separate directory and the .travis.yml file is identical the the one in this branch.

@chrisgorgo
Copy link
Member

Thanks. I started going through it but I found many unresolved conflict markers in the code. I think something must've gone wrong when you were resolving merge conflicts.

@duboism
Copy link
Contributor Author

duboism commented Sep 14, 2015

Thanks for the review (I haven't found any tool to help during the rebase so I edited the files manually hence the leftovers and various glitches). I will fix those ASAP.

@duboism
Copy link
Contributor Author

duboism commented Sep 15, 2015

The leftovers you have pointed should be corrected now.

=======
We want to connect the output produced by realignment to the input of
smoothing. This is done as follows:
>>>>>>> cb80e24fc2a68758defcb16c7ab70092aa35b693
Copy link
Member

Choose a reason for hiding this comment

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

unresolved conflict

@djarecka
Copy link
Collaborator

@duboism - it's a great pr, would you consider finishing it?

@djarecka
Copy link
Collaborator

@duboism - please let us know if you're planning to work on this PR, otherwise I'll try to review and finish/close.

@duboism
Copy link
Contributor Author

duboism commented Dec 13, 2017

@djarecka I have moved to a completely different job. I may find some time to finish this but it's not sure so feel free to work on the PR.

@djarecka
Copy link
Collaborator

@duboism - thank you for answering. We will be merging with the Nipype Tutorial, so I will review which changes will be useful in the new verion. Thanks!

@djarecka djarecka self-assigned this Dec 14, 2017
@satra
Copy link
Member

satra commented Feb 15, 2018

@djarecka - is this something that can be resolved before next release.

@djarecka
Copy link
Collaborator

I haven't reviewed this yet. Will try to it next week

@effigies effigies closed this Feb 22, 2018
effigies added a commit that referenced this pull request Feb 23, 2018
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.

8 participants