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

100 unify cost api #126

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

100 unify cost api #126

wants to merge 4 commits into from

Conversation

cmalinmayor
Copy link
Contributor

The Appear, Disappear, Node Selection, Edge Selection, and Split costs all have an optional weight that defaults to 1, an optional attribute name that defaults to None, and an optional constant that defaults to 1. Appear and Disappear also have an optional ignore_attribute that defaults to None. EdgeDistance has a position attribute (not optional), along with the same weight and constant arguments as all the other costs.

There is a lot of duplicated code, since now all the costs essentially get the indicators for a different variable type, and then do the same things to add a constant and variable costs to the objective. The appear and disappear have some special ignore functionality, but I still think we could abstract this pretty easily into a class hierarchy to avoid the duplication. Not sure it is worth the added complexity. Thoughts @funkey @tlambert03 ?

Closes #100

@cmalinmayor cmalinmayor linked an issue Nov 5, 2024 that may be closed by this pull request
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.86%. Comparing base (eb83bd2) to head (4ce3048).

Files with missing lines Patch % Lines
motile/costs/disappear.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
- Coverage   92.94%   92.86%   -0.08%     
==========================================
  Files          30       30              
  Lines         723      729       +6     
==========================================
+ Hits          672      677       +5     
- Misses         51       52       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Unify Cost API
1 participant