Cantera / cantera

Chemical kinetics, thermodynamics, and transport tool suite
https://cantera.org
Other
581 stars 342 forks source link

add __call__ method to with_units #1650

Closed mefuller closed 6 months ago

mefuller commented 6 months ago

Changes proposed in this pull request

add __call__ to cantera.with_units solutions (closes #1649)

Checklist

mefuller commented 6 months ago

@bryanwweber oops - forgot that was turned on at work - meant to get back to this anyway at home, so here is the corrected commit

Looks like this doesn't actually fix the problem and I'm not sure why (I did a scons build/test/install and I still get the same bug - any ideas as to what I did wrong (I definitely don't understand 100% what's going on here in any case)

bryanwweber commented 6 months ago

@mefuller can you open up the generated solution.py in the build folder? That'll show you the actual code being run. I also edited my comment above with a suggestion for the implementation

mefuller commented 6 months ago

@bryanwweber looks like the CI / .NET failures are due to a 404 pulling down dependencies - is there an easy way to re-trigger these?

bryanwweber commented 6 months ago

@mefuller I just restarted them

speth commented 6 months ago

I think the failure of the .NET on ubuntu-22.04 run is because we're missing a sudo apt update before the sudo apt install ... command in that workflow (though I don't really understand the circumstances under which older package versions are removed from the Ubuntu repositories).

The other errors with the .NET interface are already fixed in the main branch (see commit aee309220), and can be resolved by rebasing.

mefuller commented 6 months ago

@bryanwweber the failures in CI seem to all be on .NET (with which I am not at all familiar) - any idea what might be the cause?

bryanwweber commented 6 months ago

I'm not sure, I think some foxes are on main if you haven't rebased in a bit. Maybe @speth can help 😟

speth commented 6 months ago

The solution to both distinct errors is in my previous comment.

mefuller commented 6 months ago

Sorry7 about glossing over that @speth - rebased and rerunning

bryanwweber commented 6 months ago

Thanks @mefuller , the code looks good, I just haven't had a chance to pull it down and try it out. I'll try to do that soon!

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (87317b7) 72.68% compared to head (414d363) 72.68%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1650 +/- ## ========================================== - Coverage 72.68% 72.68% -0.01% ========================================== Files 370 370 Lines 56302 56302 Branches 20403 20403 ========================================== - Hits 40923 40922 -1 - Misses 12371 12372 +1 Partials 3008 3008 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bryanwweber commented 6 months ago

Thanks @mefuller!