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

308 documentation #312

Merged
merged 19 commits into from Oct 9, 2018

Conversation

Projects
None yet
4 participants
@boredstiff
Collaborator

boredstiff commented Aug 23, 2018

Closes #308, #304

I'm not finished and have more things I'd like to do to make it better, but I'd like to get some feedback. If you'd like to see what the documentation looks like currently, I've attached a zip file of the build, just run the index.html in your browser.

It's not perfect, I'm still working on a few things, but it's got full API generation, and the Wiki is completely migrated over. I'd like to know your thoughts before getting too far along. I'd say it's around 90% finished, though. :)

I've still got to do things like:

  • work it into travis
  • ensure codecov is still working
  • move the flake8 over to tox, it's commented out
  • add the doc testing
  • update readme with information

Additionally, in the commits, I've written in-depth descriptions so if you have any questions as to 'why', hopefully they're answered there and if not, let me know if you have more questions.

It'll probably be a couple of days before I get back to this, so take your time with feedback. I probably won't hit it again until Saturday or Sunday.

html.zip

Additionally, it's going to fail right now. I have no clue why, because in my Travis build history, every commit up until the last two were fine. My initial guess is that the flake8 settings in the tox.ini file are interfering with what Travis sees, but I'll look into that whenever I get back to working on this this weekend.

https://travis-ci.org/boredstiff/OpenTimelineIO/builds

boredstiff added some commits Aug 23, 2018

Remove old documentation files
They are out of date, and the generated ones for the API should not be checked into source control. By creating them at build time you can run tests against the documentation and ensure that the examples in the documentation are up to date with the code that has been written.

Additionally, I'm moving the doc folder over to docs in order to keep parity with the plurality of 'tests'
Add MANIFEST.in file
By adding a Manifest file, we control what will get built into the final package so that we're not adding files that don't need to go into the package on pypi. This will be run in the tox step that will be added in a commit that is coming up which will build out a temporary distribution and check that the results in the output package file match up with what we've defined in here. I'd also like to exclude the examples folder, but if I exclude it, everything in my virtualenv goes nuts and breaks and I can't figure out why and I don't feel like putting more time into that.
Migrate Wiki over to tutorials and generate all API documentation
Migrating the Wiki was actually the easy part. By adding the recommonmark sphinx plugin, I could just download the Wiki from Github and use the Markdown files that existed, with slight changes to fit into the sphinx-rtd-theme. NOTE that this isn't done or perfect, but it's 95% of the work done. So you might be wondering why one would want to move the wiki over to documentation in the project and the answer is simple - the wiki is not version controlled in alignment with the package and releases of the package.

Building the documentation along with the version of the package allows the user to see changes and explanation alongside whatever version they're using, while the Wiki does not. Yes, they will have to generate it, but with the integration of tox (coming in a separate commit), we make that so much easier on the user with the command `tox -e build-docs`. This practice also helps encourage developers to keep updating their documentation and it's worked great at the last few studios I've worked at and implemented it at.
Add tox to dev dependencies and add tox file
Since this is a new dependency, you'll have to rerun the install command with `pip install -e .[dev]`, deactivate your virtualenv, and then source it again. After that, you only have to run `tox` when you're sourced inside of your virtualenv to run all the tests, and you can run `tox -e build-docs` to build the documentation. This is not finished yet, I plan to add a couple more steps to the tox script in a coming commit, but for the purpose of running tests and building documentation, this works.

If a tox environment ever gets funky, you can run `tox -r` to rebuild the environments. I have only ever had to do that once.
Update file documentation to build API docs properly
Since I added the ability for Sphinx to build ALL of the API documentation, there were docstrings that were never touched by the build stuff that was set up previously, and thus had issues that prevented documentation from building. This was a rather annoying chore to have to go through, and I should not have messed with any code at all, it should have only been docstrings getting messed with (minus the import statements in tests). I would appreciate you all looking at this and making sure, though.
Documentation formatting updates
Adding images, formatting, fixing tables, making links work.
@ssteinbach

This comment has been minimized.

Show comment
Hide comment
@ssteinbach

ssteinbach Sep 4, 2018

Member

Cool! So it looks like you ported the wiki documents into the docs tree for the read the docs stuff very cool. Some dumb questions - the tox file, is that meant to replace the travis file? Just wondering how that fits into the puzzle. Or does it replace the make or setup.py files?

Thanks!

Member

ssteinbach commented Sep 4, 2018

Cool! So it looks like you ported the wiki documents into the docs tree for the read the docs stuff very cool. Some dumb questions - the tox file, is that meant to replace the travis file? Just wondering how that fits into the puzzle. Or does it replace the make or setup.py files?

Thanks!

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 4, 2018

Collaborator

It does not replace the setup.py file.

It would not replace the travis file either, but the travis file would call the tox file, with a prerequisite of the travis file running pip install tox in the before_install step.

It would move the steps from the make file to the tox file, as individual steps which can also be run in sequence like here

Additionally, there will be a deploy step for the documentation as well (in the travis file) that will upload the built documentation to readthedocs, on a release.

Edit: Whoops, hit the wrong hot keys, didn't mean to close it.

Collaborator

boredstiff commented Sep 4, 2018

It does not replace the setup.py file.

It would not replace the travis file either, but the travis file would call the tox file, with a prerequisite of the travis file running pip install tox in the before_install step.

It would move the steps from the make file to the tox file, as individual steps which can also be run in sequence like here

Additionally, there will be a deploy step for the documentation as well (in the travis file) that will upload the built documentation to readthedocs, on a release.

Edit: Whoops, hit the wrong hot keys, didn't mean to close it.

@boredstiff boredstiff closed this Sep 4, 2018

@boredstiff boredstiff reopened this Sep 4, 2018

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 4, 2018

Collaborator

I need to fix whatever is happening with the flake8 errors that are happening, as I don't see them when running locally. I'll also check and see if the documentation is finished, I did another pass this weekend with formatting updates. Are there any changes that you would like to see? Stuff like link formatting and table formatting should have been fixed already in commit 0bd50b0

Collaborator

boredstiff commented Sep 4, 2018

I need to fix whatever is happening with the flake8 errors that are happening, as I don't see them when running locally. I'll also check and see if the documentation is finished, I did another pass this weekend with formatting updates. Are there any changes that you would like to see? Stuff like link formatting and table formatting should have been fixed already in commit 0bd50b0

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 5, 2018

Collaborator

https://opentimelineio.readthedocs.io/en/latest/index.html

I didn't know that it would take the opentimelineio url whenever I was connecting the accounts, so I'll see if deleting it will get rid of the usage on RTD. I think it would.

Basically, you just have to connect the Github account and import the project.

Also, after a bit of work and research, I got the api documentation building for readthedocs, which is not out of the box behavior for RTD. That's everything in ab8d3c2

Collaborator

boredstiff commented Sep 5, 2018

https://opentimelineio.readthedocs.io/en/latest/index.html

I didn't know that it would take the opentimelineio url whenever I was connecting the accounts, so I'll see if deleting it will get rid of the usage on RTD. I think it would.

Basically, you just have to connect the Github account and import the project.

Also, after a bit of work and research, I got the api documentation building for readthedocs, which is not out of the box behavior for RTD. That's everything in ab8d3c2

@boredstiff boredstiff changed the title from WIP: 308 documentation to 308 documentation Sep 5, 2018

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 5, 2018

Collaborator

Almost everything is done, the only thing I don't see happening again is coverage. Will investigate.

Collaborator

boredstiff commented Sep 5, 2018

Almost everything is done, the only thing I don't see happening again is coverage. Will investigate.

@boredstiff boredstiff changed the title from 308 documentation to WIP: 308 documentation Sep 5, 2018

boredstiff added some commits Sep 6, 2018

Add coverage dependency to travis pip installation
Didn't think about the fact that it should be needed outside of the virtualenvs that Tox creats
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 6, 2018

Codecov Report

Merging #312 into master will decrease coverage by 17.12%.
The diff coverage is 96.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #312       +/-   ##
===========================================
- Coverage   90.89%   73.76%   -17.13%     
===========================================
  Files          53       59        +6     
  Lines        4512     5341      +829     
===========================================
- Hits         4101     3940      -161     
- Misses        411     1401      +990
Impacted Files Coverage Δ
opentimelineio/console/otiostat.py 72.41% <ø> (-0.09%) ⬇️
opentimelineio/opentime.py 92.6% <ø> (ø) ⬆️
opentimelineio/adapters/adapter.py 96.61% <ø> (+3.38%) ⬆️
opentimelineio/console/otioconvert.py 70.27% <ø> (ø) ⬆️
opentimelineio/schema/transition.py 87.5% <ø> (-6.25%) ⬇️
opentimelineio/core/type_registry.py 96.96% <ø> (ø) ⬆️
opentimelineio/core/item.py 97.29% <ø> (ø) ⬆️
opentimelineio/adapters/cmx_3600.py 91.56% <100%> (+0.01%) ⬆️
opentimelineio/schema/serializable_collection.py 97.56% <100%> (-2.44%) ⬇️
opentimelineio/algorithms/stack_algo.py 96.15% <100%> (ø) ⬆️
... and 22 more

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 6f15035...8f2a9c7. Read the comment docs.

codecov-io commented Sep 6, 2018

Codecov Report

Merging #312 into master will decrease coverage by 17.12%.
The diff coverage is 96.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #312       +/-   ##
===========================================
- Coverage   90.89%   73.76%   -17.13%     
===========================================
  Files          53       59        +6     
  Lines        4512     5341      +829     
===========================================
- Hits         4101     3940      -161     
- Misses        411     1401      +990
Impacted Files Coverage Δ
opentimelineio/console/otiostat.py 72.41% <ø> (-0.09%) ⬇️
opentimelineio/opentime.py 92.6% <ø> (ø) ⬆️
opentimelineio/adapters/adapter.py 96.61% <ø> (+3.38%) ⬆️
opentimelineio/console/otioconvert.py 70.27% <ø> (ø) ⬆️
opentimelineio/schema/transition.py 87.5% <ø> (-6.25%) ⬇️
opentimelineio/core/type_registry.py 96.96% <ø> (ø) ⬆️
opentimelineio/core/item.py 97.29% <ø> (ø) ⬆️
opentimelineio/adapters/cmx_3600.py 91.56% <100%> (+0.01%) ⬆️
opentimelineio/schema/serializable_collection.py 97.56% <100%> (-2.44%) ⬇️
opentimelineio/algorithms/stack_algo.py 96.15% <100%> (ø) ⬆️
... and 22 more

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 6f15035...8f2a9c7. Read the comment docs.

@boredstiff boredstiff changed the title from WIP: 308 documentation to 308 documentation Sep 6, 2018

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 6, 2018

Collaborator

This should be finished. Coverage percentage dropped, because if you look here, you can see that it's picking up new files that the make was not picking up before. I do not know why.

Changes

  • Migrate wiki documentation to sphinx-generated documentation
  • Get automated API documentation generating for Sphinx documentation
  • Update formatting to enforce pep-8 compliancy
  • Integrate tox to enable easy multi-environment testing, documentation building, documentation testing, and coverage generation.
  • Set up documentation to auto-import from readthedocs
  • Get API documentation generating for Sphinx documentation working for ReadTheDocs - quite annoying
  • Implement tox-travis to make the computational matrix systems work together
  • Update travis to work with tox instead of make
  • Clean steps out of make file

I have currently left the makefile as-is (minus updates for pep8 being renamed to pycodestyle) in order to give people time to transition. I'm more in favor of clearing out the make file, but realize that some people may want to maintain what they may be used to.

This change addresses the previously brought up issues #308, #304 of not being able to find documentation, people not knowing about the Wiki, and very importantly:

  • the wiki not being version controlled alongside the source code. Github and Gitlab's wikis are really weak because of this
  • API Documentation generation

With that, I recommend closing the wiki with this change. I have also updated the README to point to the documentation url.

Tox

Install tox into your local virtualenv that you're using with $ pip install tox

Running:

$ tox

This will run all of the steps listed in the tox file underneath the testenv step.

To run specific steps, like building the documentation locally, run

$ tox -e build-docs

This will output the documentation to .tox/build-docs/tmp/html/ where you can click index.html and open up the documentation. Additionally, you make adjustments to your documentation, and can regenerate and reload the page in the browser.

In the tox envlist, you'll notice envs called clean and stats - these are important for Travis, and is what allows us to generate and upload one file in the correct place.

Coverage

Coverage is built automatically in the testenv step. It has dropped from 90% to 72% because it's picking up new files that it was not picking up before. I didn't see a way that it was omitting those files, and they don't appear to be new files, so I'm not sure why it's picking those up now.

Dist

There is a tox step called dist which builds the zip file for you.

Manifest

The manifest was added to ensure that what is being added to the distribution that gets packaged is what is expected. You can adjust it by editing the MANIFEST.in file. I chose to omit tests, examples, documentation, PDFs, and dotfiles.

If you look in the check-manifest line, you'll see an --ignore-bad-ideas - this is because .egg-info files were checked into source control. Additionally, with the documentation that was in OTIO previously, API generated .rst files were checked into source control. Both of these fall under the same umbrella - bad ideas. I removed the .rst files from documentation and the API is generated now, which means every time you make a change, the documentation will update accordingly. I, however, left the egg-info files in there, because they appear to be needed. I didn't dig too far into that and thought it best to leave it as-is.


Feel free to ask any questions you may have.

Collaborator

boredstiff commented Sep 6, 2018

This should be finished. Coverage percentage dropped, because if you look here, you can see that it's picking up new files that the make was not picking up before. I do not know why.

Changes

  • Migrate wiki documentation to sphinx-generated documentation
  • Get automated API documentation generating for Sphinx documentation
  • Update formatting to enforce pep-8 compliancy
  • Integrate tox to enable easy multi-environment testing, documentation building, documentation testing, and coverage generation.
  • Set up documentation to auto-import from readthedocs
  • Get API documentation generating for Sphinx documentation working for ReadTheDocs - quite annoying
  • Implement tox-travis to make the computational matrix systems work together
  • Update travis to work with tox instead of make
  • Clean steps out of make file

I have currently left the makefile as-is (minus updates for pep8 being renamed to pycodestyle) in order to give people time to transition. I'm more in favor of clearing out the make file, but realize that some people may want to maintain what they may be used to.

This change addresses the previously brought up issues #308, #304 of not being able to find documentation, people not knowing about the Wiki, and very importantly:

  • the wiki not being version controlled alongside the source code. Github and Gitlab's wikis are really weak because of this
  • API Documentation generation

With that, I recommend closing the wiki with this change. I have also updated the README to point to the documentation url.

Tox

Install tox into your local virtualenv that you're using with $ pip install tox

Running:

$ tox

This will run all of the steps listed in the tox file underneath the testenv step.

To run specific steps, like building the documentation locally, run

$ tox -e build-docs

This will output the documentation to .tox/build-docs/tmp/html/ where you can click index.html and open up the documentation. Additionally, you make adjustments to your documentation, and can regenerate and reload the page in the browser.

In the tox envlist, you'll notice envs called clean and stats - these are important for Travis, and is what allows us to generate and upload one file in the correct place.

Coverage

Coverage is built automatically in the testenv step. It has dropped from 90% to 72% because it's picking up new files that it was not picking up before. I didn't see a way that it was omitting those files, and they don't appear to be new files, so I'm not sure why it's picking those up now.

Dist

There is a tox step called dist which builds the zip file for you.

Manifest

The manifest was added to ensure that what is being added to the distribution that gets packaged is what is expected. You can adjust it by editing the MANIFEST.in file. I chose to omit tests, examples, documentation, PDFs, and dotfiles.

If you look in the check-manifest line, you'll see an --ignore-bad-ideas - this is because .egg-info files were checked into source control. Additionally, with the documentation that was in OTIO previously, API generated .rst files were checked into source control. Both of these fall under the same umbrella - bad ideas. I removed the .rst files from documentation and the API is generated now, which means every time you make a change, the documentation will update accordingly. I, however, left the egg-info files in there, because they appear to be needed. I didn't dig too far into that and thought it best to leave it as-is.


Feel free to ask any questions you may have.

@jminor

This comment has been minimized.

Show comment
Hide comment
@jminor

jminor Sep 7, 2018

Collaborator

After some discussion internally and with a few other contributors, we are mostly in agreement about this PR. The new docs look really great and this will be much better than the wiki. Thanks for putting so much effort into this.

The only outstanding concern is replacing Make with tox. We are moving towards the C++ API also, so we will need to add CMake to the mix later. Since most people are more comfortable with make, we'd like to keep the Makefile working for now. We are fine with having both tox and make until we get used to it.

Collaborator

jminor commented Sep 7, 2018

After some discussion internally and with a few other contributors, we are mostly in agreement about this PR. The new docs look really great and this will be much better than the wiki. Thanks for putting so much effort into this.

The only outstanding concern is replacing Make with tox. We are moving towards the C++ API also, so we will need to add CMake to the mix later. Since most people are more comfortable with make, we'd like to keep the Makefile working for now. We are fine with having both tox and make until we get used to it.

@boredstiff

This comment has been minimized.

Show comment
Hide comment
@boredstiff

boredstiff Sep 7, 2018

Collaborator

Sounds good to me!

Collaborator

boredstiff commented Sep 7, 2018

Sounds good to me!

@ssteinbach ssteinbach added this to the Public Beta 9 milestone Sep 7, 2018

@boredstiff boredstiff referenced this pull request Sep 26, 2018

Merged

Dark Mode #321

jminor and others added some commits Oct 6, 2018

Minor fixes to new doc system (#3)
* Fixed indentation issue in doc string.

* Changed Makefile doc-html target to run new tox -e build-docs.

* Put lambda idiom back the way it was.

@jminor jminor merged commit 48c035a into PixarAnimationStudios:master Oct 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jminor

This comment has been minimized.

Show comment
Hide comment
@jminor

jminor Oct 9, 2018

Collaborator

@boredstiff thank you so much for doing all this work!

Collaborator

jminor commented Oct 9, 2018

@boredstiff thank you so much for doing all this work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment