Skip to content

211 Python dependencies #221

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

Merged
merged 5 commits into from
Apr 8, 2022
Merged

211 Python dependencies #221

merged 5 commits into from
Apr 8, 2022

Conversation

annette-lutz
Copy link
Contributor

@annette-lutz annette-lutz commented Feb 23, 2022

Merge Request - GuideLine Checklist

Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.

Checks by code author:

  • There is at least one issue associated with the pull request.
  • The branch follows the naming conventions as defined in the git workflow.
  • New code adheres with the coding guidelines
  • Tests for new functionality has been added
  • A local test was succesful
  • There is appropriate documentation of your work. (use doxygen style comments)
  • If new third party software is used, did you pay attention to its license? Please remember to add it to the wiki after successful merging.
  • If new mathematical methods or epidemiological terms are used, has the glossary been updated ? Did you provide further documentation ?
    is present or referenced. Please provide your references.
  • The following questions are addressed in the documentation*: Developers (what did you do?, how can it be maintained?), For users (how to use your work?), For admins (how to install and configure your work?)
  • For documentation: Please write or update the Readme in the current working directory!

Checks by code reviewer(s):

  • Is the code clean of development artifacts e.g., unnecessary comments, prints, ...
  • The ticket goals for each associated issue are reached or problems are clearly addressed (i.e., a new issue was introduced).
  • There are appropriate unit tests and they pass.
  • The git history is clean and linearized for the merge request.
  • Coverage report for new code is acceptable.

closes #211

@annette-lutz annette-lutz linked an issue Feb 23, 2022 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2022

Codecov Report

Merging #221 (0cbf5e6) into main (089a7e0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #221   +/-   ##
=======================================
  Coverage   91.23%   91.23%           
=======================================
  Files          77       77           
  Lines        4721     4721           
=======================================
  Hits         4307     4307           
  Misses        414      414           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 089a7e0...0cbf5e6. Read the comment docs.

@annette-lutz annette-lutz self-assigned this Feb 23, 2022
@annette-lutz annette-lutz changed the title upgrade pandas and matplotlib 211 Python dependencies Feb 23, 2022
@annette-lutz annette-lutz requested a review from xsaschako March 14, 2022 11:29
@annette-lutz annette-lutz marked this pull request as draft March 15, 2022 13:52
@annette-lutz annette-lutz marked this pull request as ready for review March 15, 2022 13:54
Copy link
Member

@xsaschako xsaschako left a comment

Choose a reason for hiding this comment

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

The CI throws an FileNotFoundError Error in the RKI File. I don't think the axis=1 does this. Do you know why it throws that error?

Else, looks good!

@dabele
Copy link
Member

dabele commented Mar 21, 2022

The CI throws an FileNotFoundError Error in the RKI File. I don't think the axis=1 does this. Do you know why it throws that error?

May be the same error that is thrown in CI on the main branch?
https://github.com/DLR-SC/memilio/runs/5621535252?check_suite_focus=true

@xsaschako
Copy link
Member

The CI throws an FileNotFoundError Error in the RKI File. I don't think the axis=1 does this. Do you know why it throws that error?

May be the same error that is thrown in CI on the main branch? https://github.com/DLR-SC/memilio/runs/5621535252?check_suite_focus=true

Looks like it. I will look into it ( maybe @annette-lutz we can discuss it tomorow morning.)

@annette-lutz
Copy link
Contributor Author

The FileNotFoundError is thrown because the link to the data from RKI is outdated. That is the case for the main branch and this branch. This is corrected in pull request #216 where the source for the rki data is changed to github and the link failing here is changed to the current link and used as a fallback variant.

@xsaschako xsaschako requested a review from dabele March 23, 2022 10:34
dabele
dabele previously approved these changes Mar 24, 2022
Copy link
Member

@dabele dabele 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 forget to properly connect PRs to issues and add a line to the description of the PR that closes the issue, "Closes #" or similar.

xsaschako
xsaschako previously approved these changes Apr 1, 2022
Copy link
Member

@xsaschako xsaschako left a comment

Choose a reason for hiding this comment

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

Looks good!

@annette-lutz annette-lutz dismissed stale reviews from xsaschako and dabele via d6a36f0 April 6, 2022 14:05
@annette-lutz annette-lutz requested a review from xsaschako April 6, 2022 14:06
Copy link
Member

@xsaschako xsaschako left a comment

Choose a reason for hiding this comment

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

Looks good!

@xsaschako xsaschako merged commit 8d14d35 into main Apr 8, 2022
@xsaschako xsaschako deleted the 211-python-dependencies branch April 8, 2022 11:29
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.

Python dependencies: Upgrade pandas and matplotlib, check xlrd
4 participants