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

Fix#445 #455

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

Fix#445 #455

wants to merge 5 commits into from

Conversation

darribas
Copy link
Member

@darribas darribas commented Jan 29, 2022

Hello! Please make sure to check all these boxes before submitting a Pull Request
(PR). Once you have checked the boxes, feel free to remove all text except the
justification in point 5.

  1. You have run tests on this submission locally using pytest on your changes. Continuous integration will be run on all PRs with GitHub Actions, but it is good practice to test changes locally prior to a making a PR.
  2. This pull request is directed to the pysal/master branch.
  3. This pull introduces new functionality covered by
    docstrings and
    unittests?
  4. You have assigned a
    reviewer
    and added relevant labels
  5. The justification for this PR is: Fix Add support for DataArray objects read through rioxarray #445

@darribas darribas added the bug functionality that: returns invalid, erroneous, or meaningless results; or doesn't work at all. label Jan 29, 2022
@darribas darribas requested a review from ljwolf January 29, 2022 17:06
Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Can you try to increase test coverage to cover more of these options? Codecov reports some of these lines as unchecked currently.

libpysal/weights/raster.py Outdated Show resolved Hide resolved
Co-authored-by: Martin Fleischmann <[email protected]>
@codecov
Copy link

codecov bot commented Jan 30, 2022

Codecov Report

Merging #455 (ea8e4af) into master (8aa08b1) will increase coverage by 0.1%.
The diff coverage is 81.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #455     +/-   ##
========================================
+ Coverage    78.7%   78.8%   +0.1%     
========================================
  Files         122     122             
  Lines       12884   12898     +14     
========================================
+ Hits        10142   10163     +21     
+ Misses       2742    2735      -7     
Impacted Files Coverage Δ
libpysal/weights/raster.py 58.6% <81.0%> (+1.4%) ⬆️
libpysal/_version.py 40.7% <0.0%> (+2.7%) ⬆️

@ljwolf
Copy link
Member

ljwolf commented Mar 3, 2022

What does this need to merge?

@martinfleis
Copy link
Member

We should test the use case from #445 this is supposed to fix. It is not being covered by current tests.

@martinfleis martinfleis changed the base branch from master to main February 27, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug functionality that: returns invalid, erroneous, or meaningless results; or doesn't work at all.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for DataArray objects read through rioxarray
4 participants