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

ruff format repo #156

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Conversation

jGaboardi
Copy link
Member

@jGaboardi jGaboardi commented Sep 1, 2024

This MR:

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 57.81041% with 316 lines in your changes missing coverage. Please review.

Project coverage is 67.6%. Comparing base (d92edcb) to head (8a0969d).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
spreg/dgp.py 17.9% 69 Missing ⚠️
spreg/error_sp_regimes.py 48.6% 35 Missing and 1 partial ⚠️
spreg/error_sp.py 59.3% 20 Missing and 2 partials ⚠️
spreg/error_sp_het.py 48.6% 18 Missing and 1 partial ⚠️
spreg/utils.py 25.0% 16 Missing and 2 partials ⚠️
spreg/error_sp_hom.py 65.8% 12 Missing and 1 partial ⚠️
spreg/ols_regimes.py 38.1% 13 Missing ⚠️
spreg/error_sp_hom_regimes.py 72.7% 11 Missing and 1 partial ⚠️
spreg/twosls_sp_regimes.py 65.7% 11 Missing and 1 partial ⚠️
spreg/error_sp_het_regimes.py 75.0% 10 Missing and 1 partial ⚠️
... and 16 more
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #156   +/-   ##
=====================================
  Coverage   67.6%   67.6%           
=====================================
  Files         41      41           
  Lines       9622    9622           
  Branches    1433    1433           
=====================================
  Hits        6504    6504           
  Misses      2704    2704           
  Partials     414     414           
Files with missing lines Coverage Δ
spreg/diagnostics.py 87.0% <100.0%> (ø)
spreg/diagnostics_panel.py 96.0% <100.0%> (ø)
spreg/diagnostics_sp.py 97.0% <100.0%> (ø)
spreg/diagnostics_sur.py 100.0% <100.0%> (ø)
spreg/diagnostics_tsls.py 87.9% <ø> (ø)
spreg/panel_fe.py 94.5% <ø> (ø)
spreg/panel_utils.py 49.2% <ø> (ø)
spreg/regimes.py 81.1% <100.0%> (ø)
spreg/sp_panels.py 87.0% <ø> (ø)
spreg/sur.py 76.8% <ø> (ø)
... and 27 more

@knaaptime
Copy link
Member

this looks like a lot of changes, but it's nothing but formatting. Everything's passing. Will wait for @pedrovma's approval, but this is ready to merge

@jGaboardi
Copy link
Member Author

@pedrovma After this PR is reviewed and merged (like @knaaptime mentioned, it's only formatting changes), I will get started with #155, which will be more involved. However, both standardized formatting and implementing linting will result in a much more maintainable codebase.

@pedrovma
Copy link
Member

pedrovma commented Sep 2, 2024

This is great. The issue here is just that several of these files have already been modified in the private GeoDa repo, so some of the new formatting may be lost. But I will do my best to merge them appropriately!
Many thanks, @jGaboardi!

@jGaboardi
Copy link
Member Author

have already been modified in the private GeoDa repo

One thing you can do is also run ruff format spreg on the private repo side of things. That will make merging in much less painful later.

@martinfleis
Copy link
Member

Just a question. Why do you have a private repo instead of developing in public like we do in any other package? It is super hard to make contributions to spreg if we don't know what is the current state of the codebase.

@lanselin
Copy link
Member

lanselin commented Sep 2, 2024

That's by design. Pedro and I made that decision a long time ago to make development more efficient. The focus is on adding new methods and that is done most efficiently between the two of us without the overhead of the standard GitHub protocol. We're the gate keepers, so to speak. When the code is ready, it goes to the open repo, but for any substantive changes/contributions, people should contact us first.

@martinfleis
Copy link
Member

Fair.

for any substantive changes/contributions, people should contact us first.

Shall we add a note on that to the Readme, so the contributing process is clear?

@lanselin
Copy link
Member

lanselin commented Sep 2, 2024

Sure, great idea.

@knaaptime knaaptime merged commit f50f0b1 into pysal:main Sep 2, 2024
10 of 11 checks passed
@jGaboardi jGaboardi deleted the GH150_update_formatting branch September 2, 2024 19:14
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.

update pre-commit -- add ruff ; drop black swap from black to ruff for formatting
5 participants