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

Updating ciceroscmpy version and tests this also gets rid of pandas <… #89

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

maritsandstad
Copy link
Collaborator

…2 requirement

@maritsandstad
Copy link
Collaborator Author

Should I try to up the test coverage for my various help functions in this while I'm at it @znicholls ?

(Otherwise this might just be a quick update version thing just to get less outdated versioning...)

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b3c8cbf) 70.38% compared to head (2c73e06) 70.38%.
Report is 1 commits behind head on master.

Files Patch % Lines
...dapters/utils/cicero_utils/make_scenario_common.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master      #89   +/-   ##
=======================================
  Coverage   70.38%   70.38%           
=======================================
  Files          36       36           
  Lines        1442     1442           
  Branches      194      194           
=======================================
  Hits         1015     1015           
  Misses        405      405           
  Partials       22       22           

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

@znicholls
Copy link
Collaborator

Should I try to up the test coverage for my various help functions in this while I'm at it @znicholls ?

If you want, can also be done in a follow up PR to keep the scope of each smaller

@maritsandstad
Copy link
Collaborator Author

Seems I need to spend some time fighting with windows first, anyways...

@znicholls
Copy link
Collaborator

Seems I need to spend some time fighting with windows first, anyways...

The error is really bizarre. The CI shows the environment at the top in case that's helpful (maybe different package versions available on windows compared to linux...)

e.g. CI https://github.com/openscm/openscm-runner/actions/runs/7708984548/job/21009201207?pr=89

If you scroll to the right bit you see

Screen Shot 2024-01-30 at 11 39 01 am

@maritsandstad
Copy link
Collaborator Author

Ok, fixed the windows thing, apparently windows default int type for numpy is np.int32... Anyways, the fail now is coverage, so if the tests are supposed to go green, then I might need to work on that...

Copy link
Collaborator

@znicholls znicholls left a comment

Choose a reason for hiding this comment

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

Don't worry about the coverage, that's just coverage being silly (I'll just override it when I merge).

If you can mark the PR as ready for review and make the updates then I'll merge assuming everything goes green (once it's no longer a draft, a couple more things get run which may flag further errors).

@maritsandstad maritsandstad marked this pull request as ready for review January 30, 2024 12:49
@maritsandstad
Copy link
Collaborator Author

Ok, think that covers the rest of the updates :-)

@znicholls znicholls merged commit 7667205 into master Jan 30, 2024
18 of 19 checks passed
@znicholls znicholls deleted the update-versions-ciceroscm branch January 30, 2024 13:09
@znicholls znicholls mentioned this pull request Jan 30, 2024
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.

2 participants