Skip to content

Add Flow Refueling Location Model (FRLM)#487

Merged
ljwolf merged 14 commits intopysal:mainfrom
fengzixin0617:add-frlm-feature
Sep 11, 2025
Merged

Add Flow Refueling Location Model (FRLM)#487
ljwolf merged 14 commits intopysal:mainfrom
fengzixin0617:add-frlm-feature

Conversation

@fengzixin0617
Copy link
Copy Markdown
Contributor

  • Add FRLM class and example notebook
  • With exact and greedy solver options
  • Combined capacited FRLM and basic FRLM

Copy link
Copy Markdown
Member

@ljwolf ljwolf left a comment

Choose a reason for hiding this comment

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

Overall, great first pass! We need some more documentation throughout, and we need to consider this in light of the naming conventions and API style of the other problems.

See the comments across the PR for advice.

Comment thread notebooks/frlm.ipynb Outdated
Comment thread notebooks/frlm.ipynb Outdated
Comment thread notebooks/frlm.ipynb Outdated
Comment thread notebooks/frlm.ipynb Outdated
Comment thread notebooks/frlm.ipynb Outdated
Comment thread spopt/locate/frlm.py Outdated
Comment thread spopt/locate/frlm.py Outdated
Comment thread spopt/locate/frlm.py Outdated
Comment thread spopt/locate/frlm.py Outdated
Comment thread spopt/locate/frlm.py Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 17, 2025

Codecov Report

❌ Patch coverage is 73.27236% with 263 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.9%. Comparing base (13ca45e) to head (9b36063).
⚠️ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
spopt/locate/flow.py 73.2% 259 Missing ⚠️
spopt/locate/util.py 77.8% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #487     +/-   ##
=======================================
- Coverage   77.8%   76.9%   -0.9%     
=======================================
  Files         27      29      +2     
  Lines       2638    3700   +1062     
=======================================
+ Hits        2053    2846    +793     
- Misses       585     854    +269     
Files with missing lines Coverage Δ
spopt/locate/__init__.py 100.0% <100.0%> (ø)
spopt/locate/util.py 92.2% <77.8%> (-7.8%) ⬇️
spopt/locate/flow.py 73.2% <73.2%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
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.

Great work! Some comments about the code. I have not reviewed the whole file but I think it is good for this round. :)

Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/util.py Outdated
Copy link
Copy Markdown
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.

Wow! This is brilliant.
Just minor comments. In summary, the code is much cleaner and organized. Great work!!!!!!

Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Comment thread spopt/locate/flow.py Outdated
Copy link
Copy Markdown
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.

LGTM 🎩

@ljwolf ljwolf self-requested a review September 11, 2025 09:44
@ljwolf
Copy link
Copy Markdown
Member

ljwolf commented Sep 11, 2025

Yes, I'm also fine to ship 😄 super work @fengzixin0617

@ljwolf ljwolf merged commit 8314f1b into pysal:main Sep 11, 2025
11 checks passed
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.

4 participants