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

MRG, DOC: Integrate manual parts #6767

Merged
merged 9 commits into from
Sep 19, 2019
Merged

MRG, DOC: Integrate manual parts #6767

merged 9 commits into from
Sep 19, 2019

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Sep 13, 2019

Okay @drammock I got a good start on integrating the manual parts. Can you look into the last few points, and also do a general cleanup according to whatever you think helps the organization the most?

  • Remove all c_legacy prefixes by going through the files, modernizing them, and updating refs
  • Move manual/ stuff to overview/algorithms/
  • Make the simple ICA manual file a brief discussions tutorial
  • Remove MNE-C docs
  • Incorporate sample_dataset.rst information directly into datasets_index.rst
  • Incorporate what is in this PR ssp-api.rst content into a tutorial (and/or what in this PR is ssp.rst) and remove the ssp-api.rst file
  • Upload PDF of MNE-C manual (probably directly to mne-tools.github.io repo somewhere?) and update the currently broken link in doc/links.inc

Don't look at the diff too much, it's insane. Better to just browse the result and iterate.

I would like to get this into 0.19 if possible. Do you think it's reasonable for you to get the rest done early next week? If not I can try to spend more time on it.

@larsoner larsoner added this to the 0.19 milestone Sep 13, 2019
@drammock
Copy link
Member

@larsoner I can prioritize this for Monday

@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #6767 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #6767      +/-   ##
=========================================
+ Coverage   89.53%   89.6%   +0.06%     
=========================================
  Files         421     421              
  Lines       76301   76177     -124     
  Branches    12478   12450      -28     
=========================================
- Hits        68319   68261      -58     
+ Misses       5161    5112      -49     
+ Partials     2821    2804      -17

@drammock
Copy link
Member

@larsoner I think we should punt on integrating the cookbook at this point. It will be a fair amount of work that I don't think can be done well in time for the 0.19 release.

See also mne-tools/mne-tools.github.io#17

@larsoner
Copy link
Member Author

So then do you think this is good enough as is for 0.19?

@larsoner larsoner changed the title WIP, DOC: Integrate manual parts MRG, DOC: Integrate manual parts Sep 16, 2019
@drammock
Copy link
Member

I think it's good enough that if you want to merge today, you could. If it can wait one more day I would spend a little time tomorrow smoothing out the migrating page and maybe tweaking the overview page a tiny bit.

@larsoner
Copy link
Member Author

I'm fine to wait another day

@lgtm-com
Copy link

lgtm-com bot commented Sep 16, 2019

This pull request introduces 3 alerts when merging 639771a into df66846 - view on LGTM.com

new alerts:

  • 3 for Unhashable object hashed

@larsoner larsoner changed the title MRG, DOC: Integrate manual parts WIP, DOC: Integrate manual parts Sep 17, 2019
@larsoner
Copy link
Member Author

@drammock marked as WIP, feel free to set to MRG once you're happy and I'll take another look, and then if we can get another +1 we should be good for 0.19.

@larsoner larsoner mentioned this pull request Sep 17, 2019
@larsoner
Copy link
Member Author

I pushed a PEP8 fix right after yours @drammock

@drammock
Copy link
Member

@larsoner I just noticed that the MNE-MATLAB docs got inserted into the MNE-C install pages, was that intentional?

@drammock drammock changed the title WIP, DOC: Integrate manual parts MRG, DOC: Integrate manual parts Sep 17, 2019
Copy link
Member Author

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

LGTM +1 for merge. The overview page looks nice:

https://15417-1301584-gh.circle-artifacts.com/0/dev/overview/index.html

@jasmainak
Copy link
Member

@larsoner @drammock there used to be a page about IO, has that been nuked now? I can find it through the search bar but not through any weblinks.

@drammock
Copy link
Member

@jasmainak the page still exists (as you found by searching) but is not very discoverable. This is a known issue; it's true of everything in doc/overview/algorithms at the moment. In a subsequent PR we'll clean those up and make them easier to find, and also integrate the "cookbook" page into the rest of the docs (possibly as a tutorial, or divvy up its content into several tutorials). I'll be opening some doc TODO issues today in prep for the sprint; this will be one of them.

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

feel free to merge

@drammock
Copy link
Member

merging as we have approvals from @larsoner and @agramfort and me.

@drammock drammock merged commit ff349f3 into mne-tools:master Sep 19, 2019
@larsoner larsoner deleted the manual branch September 19, 2019 20:19
alexrockhill pushed a commit to alexrockhill/mne-python that referenced this pull request Oct 1, 2019
* DOC: Integrate manual parts

* integrate SSP-api into tutorial

* add URL for MNE-C manual PDF

* fix: missing newline at EOF breaks RST rendering?

* fix dead crossref

* cleanups/tweaks

* STY: PEP8

* typos

* [skip travis][skip azp] tweaks to install doc sequence
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.

4 participants