Skip to content

Comments

Fix error message in scipy_match_demand#401

Merged
tsmbland merged 2 commits intodevelopfrom
fix_error_message
Jul 5, 2024
Merged

Fix error message in scipy_match_demand#401
tsmbland merged 2 commits intodevelopfrom
fix_error_message

Conversation

@tsmbland
Copy link
Collaborator

@tsmbland tsmbland commented Jul 4, 2024

I just realised that one of the changes I made in #381 broke an error message in the scipy_match_demand function. This fixes it

@tsmbland tsmbland requested a review from dalonsoa July 4, 2024 20:14
@tsmbland
Copy link
Collaborator Author

tsmbland commented Jul 4, 2024

Or not...

@dalonsoa
Copy link
Collaborator

dalonsoa commented Jul 5, 2024

I'm not sure I understand. Except for the xfail test, that PR was passing all checks, so what was broken?

@tsmbland
Copy link
Collaborator Author

tsmbland commented Jul 5, 2024

It's this error message, which is raised when the solver can't find a solution (which is supposed to give a nice message but now just complains that df_technologies doesn't have "technology"). It's not part of the testing suite, so I only noticed when dealing with an infeasible model. Obviously my first attempt to fix it hasn't worked (although strangely it works for me locally), so I'll try something else

@codecov
Copy link

codecov bot commented Jul 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.36%. Comparing base (06d35af) to head (ebb816a).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #401      +/-   ##
===========================================
- Coverage    71.36%   71.36%   -0.01%     
===========================================
  Files           44       44              
  Lines         5916     5915       -1     
  Branches      1162     1162              
===========================================
- Hits          4222     4221       -1     
  Misses        1371     1371              
  Partials       323      323              

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

@tsmbland
Copy link
Collaborator Author

tsmbland commented Jul 5, 2024

Ok now it looks better

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

Assuming the failing windows check isn't relevant, it looks good!


from muse.constraints import ScipyAdapter

df_technologies = technologies.to_dataframe()
Copy link
Collaborator

Choose a reason for hiding this comment

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

So... is this not needed anymore?

Copy link
Collaborator Author

@tsmbland tsmbland Jul 5, 2024

Choose a reason for hiding this comment

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

Nope. Previously it was converting the xarray to a dataframe and then getting the list of technologies from the dataframe, but you can get this directly from the xarray (and the dataframe isn't used anywhere else)

@tsmbland tsmbland merged commit d91476d into develop Jul 5, 2024
@tsmbland tsmbland deleted the fix_error_message branch July 5, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants