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

LSCP-B #259

Merged
merged 71 commits into from
Aug 4, 2022
Merged

LSCP-B #259

merged 71 commits into from
Aug 4, 2022

Conversation

erinrolson
Copy link
Contributor

@erinrolson erinrolson commented Jun 28, 2022

Implements LSCPB class for GSoC '22

@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Merging #259 (6888fcc) into main (57b3abc) will increase coverage by 1.1%.
The diff coverage is 96.2%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #259     +/-   ##
=======================================
+ Coverage   66.1%   67.2%   +1.1%     
=======================================
  Files         23      23             
  Lines       2582    2659     +77     
  Branches     566     583     +17     
=======================================
+ Hits        1706    1787     +81     
+ Misses       790     781      -9     
- Partials      86      91      +5     
Impacted Files Coverage Δ
spopt/locate/base.py 94.4% <84.6%> (+0.5%) ⬆️
spopt/locate/coverage.py 99.0% <98.5%> (-0.2%) ⬇️
spopt/_version.py 33.8% <0.0%> (+1.5%) ⬆️

Copy link
Member

@jGaboardi jGaboardi left a comment

Choose a reason for hiding this comment

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

Phenomenal work, @erinrolson! This is a great start here! I think next we need to think about:

  • adding in tests for the LSCP-B; and
  • creating a notebook demonstrating the LSCP-B

spopt/locate/coverage.py Outdated Show resolved Hide resolved
spopt/locate/coverage.py Outdated Show resolved Hide resolved
spopt/locate/coverage.py Outdated Show resolved Hide resolved
spopt/locate/coverage.py Show resolved Hide resolved
@jGaboardi
Copy link
Member

cc @qszhao

@jGaboardi
Copy link
Member

@erinrolson Let's remove the .coverage file from this PR. This file should actually be on the .gitignore list, but we seem to have left it out.

@jGaboardi jGaboardi self-requested a review July 27, 2022 13:52
@jGaboardi
Copy link
Member

@erinrolson I think after removing the .coverage file from this PR it will be ready for approval.

Copy link
Member

@gegen07 gegen07 left a comment

Choose a reason for hiding this comment

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

  • Could you run the lscpb notebook from the beginning? It starts from 30th cell.
  • Maybe we can add the plot_results function to util file? We are using it in all notebooks. @jGaboardi what do you think?

@erinrolson just small changes to wrap up this awesome work!

spopt/locate/coverage.py Outdated Show resolved Hide resolved
spopt/locate/coverage.py Outdated Show resolved Hide resolved
@jGaboardi
Copy link
Member

Maybe we can add the plot_results function to util file? We are using it in all notebooks. @jGaboardi what do you think?

@gegen07 This could be a nice idea (though not for this PR). We may also discuss integrating viz functionality with splot.

@gegen07
Copy link
Member

gegen07 commented Aug 4, 2022

Thanks @erinrolson for your awesome contribution!! 🎉

@gegen07 gegen07 merged commit 768ea8a into pysal:main Aug 4, 2022
@erinrolson erinrolson deleted the lscp-b branch August 4, 2022 17:10
@jGaboardi jGaboardi added enhancement New feature or request locate GSoC2022 labels Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request GSoC2022 locate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants