Skip to content

docs: refactor documentation #23

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

Merged

Conversation

nikolay-bushkov
Copy link
Contributor

Autoconversion of docstrings with pyment doesn't work well, because the initial format was not following a strict standard. So there are a lot of manual corrections. I have chosen Google style for docstring, however conversion from it to NumPy style with pyment could be easier.

The first half of modAL.models looks good, but there may be some improvements (further deduplication) in coming days. Review and comments on committed parts could help to finish the whole refactoring (I hope, by the weekend).

@nikolay-bushkov
Copy link
Contributor Author

#22

@cosmic-cortex
Copy link
Member

Thanks, awesome! I'll review the commit ASAP.

@cosmic-cortex
Copy link
Member

I have reviewed the changes and made a few commits myself. Shall I push them to the corresponding branch in your fork or do you prefer for me to push the commits to a new feature branch here?

Aside from some minor changes, something happened in the models.py file which caused some trouble with the BaseCommittee, Committee, CommitteeRegressor classes to be indented one level further. I have fixed this and now the code passes all tests.

@nikolay-bushkov
Copy link
Contributor Author

Oh... Haven't notice these indents, and Travis didn't show test result for PR... It is better if you commit directly to my fork. You should have write access to the branch, but anyway I add you as a collaborator.

@codecov-io
Copy link

codecov-io commented Sep 27, 2018

Codecov Report

Merging #23 into dev will decrease coverage by 0.04%.
The diff coverage is 98.36%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev     #23      +/-   ##
=========================================
- Coverage   97.44%   97.4%   -0.05%     
=========================================
  Files          26      26              
  Lines        1333    1350      +17     
=========================================
+ Hits         1299    1315      +16     
- Misses         34      35       +1
Impacted Files Coverage Δ
modAL/utils/__init__.py 100% <100%> (ø) ⬆️
modAL/models.py 91.97% <100%> (+0.17%) ⬆️
modAL/utils/combination.py 100% <100%> (ø) ⬆️
modAL/utils/validation.py 100% <100%> (ø) ⬆️
modAL/density.py 100% <100%> (ø) ⬆️
modAL/utils/data.py 92.3% <83.33%> (-7.7%) ⬇️

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 c1aa878...af941ca. Read the comment docs.

@cosmic-cortex
Copy link
Member

cosmic-cortex commented Sep 27, 2018

I have pushed the commits. In addition to some minor changes like fixing headers and broken links, I have removed the link to Ranked-batch-queries.rst for now. The page is not written yet.

Also, I have realized that replacing all pages with Jupyter notebooks is a good idea, so thanks for suggesting to use notebooks in the docs! I believe it would make the documentation more accessible and interactive.

In addition, I'll try to set up Travis CI for all branches. Now it checks only pushes to master and dev, but I clearly see now that this is not sufficient.
The Travis runs the build for pull requests also, but for some reason, it is not shown here.

@nikolay-bushkov nikolay-bushkov changed the title docs: refactor the first half of modAL.models docs: refactor documentation Sep 28, 2018
@cosmic-cortex
Copy link
Member

I'll review the changes hopefully today, merge them to the master branch and make a new release during the weekend. Thank you for your work! The documentation and the website is much better than it was before!

@cosmic-cortex cosmic-cortex merged commit af941ca into modAL-python:dev Sep 29, 2018
@nikolay-bushkov
Copy link
Contributor Author

Actually, I haven't finished with docstrings yet... I will try to do that tomorrow (in the next pull request)... Hope to finish before the release :)

@cosmic-cortex
Copy link
Member

Yeah, I was too fast :) I'll wait for you, no need to rush with the new release.

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.

3 participants