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

standardizing libpysal/weights docs #319

Open
wants to merge 58 commits into
base: main
Choose a base branch
from

Conversation

jGaboardi
Copy link
Member

@jGaboardi jGaboardi commented Jul 14, 2020

This PR focuses on standardizing the libpysal/weights/* docs. See #290, #291, pysal/pysal#1174.

@sjsrey This is ready for review. It is 95% docstring standardization and 5% logic/code streaming. I have also added several functions to the api docs that had been left out earlier.

Shall I rebuild the actual doc html files or leave that until #290 is fully complete (or even next release)?

@jGaboardi jGaboardi added WIP Work in progress, do not merge. Discussion only. docs labels Jul 14, 2020
@jGaboardi jGaboardi added this to the 4.4.0 milestone Jul 14, 2020
@jGaboardi jGaboardi self-assigned this Jul 14, 2020
@codecov
Copy link

codecov bot commented Jul 14, 2020

Codecov Report

Merging #319 (c8c27b1) into main (8116db6) will increase coverage by 0.1%.
The diff coverage is 69.6%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #319     +/-   ##
=======================================
+ Coverage   79.9%   80.0%   +0.1%     
=======================================
  Files        122     122             
  Lines      13225   13275     +50     
=======================================
+ Hits       10563   10614     +51     
+ Misses      2662    2661      -1     
Impacted Files Coverage Δ
libpysal/cg/ops/_accessors.py 80.0% <ø> (ø)
libpysal/cg/ops/_shapely.py 46.7% <ø> (ø)
libpysal/cg/ops/tabular.py 55.8% <ø> (ø)
libpysal/cg/ops/tests/test_shapely.py 28.8% <ø> (ø)
libpysal/cg/polygonQuadTreeStructure.py 85.7% <ø> (ø)
libpysal/cg/rtree.py 91.0% <ø> (ø)
libpysal/cg/sphere.py 72.6% <ø> (ø)
libpysal/cg/standalone.py 91.5% <ø> (ø)
libpysal/cg/tests/test_geoJSON.py 97.2% <ø> (ø)
libpysal/cg/tests/test_locators.py 96.2% <ø> (ø)
... and 62 more

@jGaboardi
Copy link
Member Author

jGaboardi commented Oct 25, 2020

@sjsrey This PR is ready for review. Windows failures for Python 3.6 only due to the current issue upstream with fiona. This PR is independent of that issue.

@jGaboardi
Copy link
Member Author

@sjsrey All tests green again.

@martinfleis
Copy link
Member

I have no clue how to review this beast.

@jGaboardi I know it is late now but it would be better to split changes of docstrings from blackening the code to different PRs to know what to look for in the review. This is just impossible to review at this point.

Are there any changes to look for? Anything code-related that is not linting?

@jGaboardi
Copy link
Member Author

I have no clue how to review this beast.

@jGaboardi I know it is late now but it would be better to split changes of docstrings from blackening the code to different PRs to know what to look for in the review. This is just impossible to review at this point.

Are there any changes to look for? Anything code-related that is not linting?

Yeah, this thing got a tad unwieldy over the years. I will looking into splitting it up as time permits. Probably start with black only in one PR then possibly even ruff (though I suspect ruff will catch some more-than-minor stuff in our code base here...). After that, I'll look back into the docstring standardization. Let's leave this PR open for now until all the constituent pieces are merged (or rejected).

@martinfleis
Copy link
Member

Okay. Try to split black and ruff into different PRs. Ruff tends to involve code changes that need review while black can be merged directly.

@jGaboardi
Copy link
Member Author

Yep, that was my plan. Maybe even ruff only 1 or 2 files at a time depending on the amount of changes made.

@martinfleis
Copy link
Member

You will have to do ruff for the whole codebase to make it pass CI

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

Successfully merging this pull request may close these issues.

3 participants