Skip to content

Update docs#445

Merged
adelavega merged 44 commits intomasterfrom
update_docs
Mar 24, 2021
Merged

Update docs#445
adelavega merged 44 commits intomasterfrom
update_docs

Conversation

@adelavega
Copy link
Copy Markdown
Member

@adelavega adelavega commented Feb 27, 2021

Closes #402

I updated the docs to reflect new features added in the last few releases. I didn't make any other substantive changes to the docs, a maintenance update.

In a previous commit (already in master), I added GH Actions to build docs & upload as an artifact for PRs, and upload to gh-pages when merged to master.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 27, 2021

Codecov Report

Merging #445 (95aff5a) into master (244d08f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #445   +/-   ##
=======================================
  Coverage   75.73%   75.73%           
=======================================
  Files          65       65           
  Lines        3820     3820           
=======================================
  Hits         2893     2893           
  Misses        927      927           
Impacted Files Coverage Δ
pliers/extractors/api/clarifai.py 23.01% <ø> (ø)
pliers/extractors/text.py 97.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 244d08f...95aff5a. Read the comment docs.

@adelavega
Copy link
Copy Markdown
Member Author

adelavega commented Feb 27, 2021

@rbroc do you mind taking a look at the docs to see if theres anything you want to add or improve? just wanted to get the ball rolling.

@rbroc
Copy link
Copy Markdown
Collaborator

rbroc commented Mar 2, 2021

had a look and pushed a couple of fixes to typos/issues with docstrings.
I think we may still be missing:

  • AudioResamplingFilter in the Filters section.
  • All the misc-type stuff (SeriesStim, MetricExtractor, ExtractorResultToSeriesConverter).
    The converter could deserve a little paragraph of its own (either in the converters section, or where Result objects are discussed). Not sure what _replacements.rst does, but a few extractors are missing there too.

Re: tutorials and quickstart (though not relevant to this PR), probably worth developing a section for Bert, AudioSet and other DL models where complexity scales a bit. Probably also an example with MetricExtractor would be beneficial.

Let's touch base on these points later, happy to help with fixes to docs and/or tutorials. :)

@adelavega
Copy link
Copy Markdown
Member Author

Yes, I think that's what's missing! Feel free to take a stab if you have some free cycles.

@adelavega
Copy link
Copy Markdown
Member Author

@rbroc have you had any time to work on the changes you wanted to make?

@rbroc
Copy link
Copy Markdown
Collaborator

rbroc commented Mar 16, 2021

done now - maybe do take a quick look just to make sure I did it right ;)

@adelavega
Copy link
Copy Markdown
Member Author

Roberta, looks good, however now I'm realizing something is going wrong w/ Sphinx. The substitutions are failing at sometimes that I wouldn't expect them to.

Also, the autodefinition tables are blank.

Take a look at the artifact: https://github.com/PsychoinformaticsLab/pliers/suites/2267564946/artifacts/47272079

I'm not sure what's going on, but some of this stems from the prior PR it seems, so perhaps its a more general issue with the GH Actions.

Anyone have any clues? @tyarkoni ?

@adelavega
Copy link
Copy Markdown
Member Author

See how the autodoc is yielding empty docstrings: http://psychoinformaticslab.github.io/pliers/reference.html#module-pliers.extractors

@adelavega
Copy link
Copy Markdown
Member Author

Okay, that was a pain but I fixed it. Sphinx could not find pliers. had to remake the entire action w/ custom code but now we're not relying on an obscure github action that wasn't doing much in the first place.

still a few more minor sphinx errors to be fixed but otherwise, this should be close

@@ -1,15 +1,9 @@
# pliers
Copy link
Copy Markdown
Contributor

@jdkent jdkent Mar 19, 2021

Choose a reason for hiding this comment

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

If you want to be even more radical (and recycle the README.md in the docs, you can convert the readme to rst and include the README.rst in index.rst like fmriprep does

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's basically what I was too lazy to do but its a good idea

Co-authored-by: James Kent <jamesdkent21@gmail.com>
README.rst Outdated
(possibly platform-dependent) requirements. For example, python-magic
requires libmagic and without this, you’ll be relegated to loading all
your stims explicitly rather than passing in filenames (i.e.,
``stim = VideoStim('my_video.mp4')`` will work fine, but passi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it does look like the README is cut off?!?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wild. I've always suspected @adelavega doesn't know how to operate copy and paste ;)

Copy link
Copy Markdown
Member Author

@adelavega adelavega Mar 24, 2021

Choose a reason for hiding this comment

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

heh, actually I blame the markdown -> rst converter @jdkent gave me

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds right, must be a limit on the website, I guess you need to run pandoc locally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did now. Now something seems to be failing w/ sphinx grr

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that's odd: something to do with this changing?
success: https://github.com/PsychoinformaticsLab/pliers/runs/2169953211?check_suite_focus=true#step:5:15
failure: https://github.com/PsychoinformaticsLab/pliers/runs/2186864097#step:5:14

I wonder why that changed? would manually adding /home/runner/.local/bin to PATH fix it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That did it. Weird.

@adelavega adelavega merged commit b01e1e9 into master Mar 24, 2021
@adelavega adelavega deleted the update_docs branch March 24, 2021 22:05
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.

update documention

5 participants