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

r.connectivity.distance bugfix #1091

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

Conversation

ecodiv
Copy link
Contributor

@ecodiv ecodiv commented May 31, 2024

  • Bugfix resulting in error when using the -p flag
  • Correction in example on manual page
  • Some small code cleanup

question: can from osgeo import gdal, osr, ogr be replaced by from osgeo import ogr? The gdal and osr do not seem to be used?

@ecodiv ecodiv requested a review from ninsbl May 31, 2024 15:51
@ninsbl
Copy link
Member

ninsbl commented Jun 3, 2024

Hi, @ecodiv and sorry for the inconvenience. I do have a major update of the module that includes parallel processing. However, when testing my update again, I discovered a bug in v.db.join in 8.4. I plan to post a fix for v.db.join tonight and then create a PR for the new r.connectivity.distance

@ecodiv
Copy link
Contributor Author

ecodiv commented Jun 3, 2024

I'm having trouble getting rpy2 to work on my computer so I was just looking into how difficult it would be to replace the R part in the second module with Python. Is that something which you are (still) considering to do as well?

@ninsbl
Copy link
Member

ninsbl commented Jun 3, 2024

Yes, The R implementation is buggy and not as performant as I wish it would be. I do have a partial replacement of R with Python for r.connectivity.network. Will have a look into it and at least provide you with what I have so far (as I am currently not working on ecology topics...).

@ecodiv
Copy link
Contributor Author

ecodiv commented Jun 3, 2024

Thanks, that would be great.

Comment on lines 32 to 33
<li>an edge-map (connections between patches) and a</li>
<li>vertex-map (centroid representations of the patches).</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<li>an edge-map (connections between patches) and a</li>
<li>vertex-map (centroid representations of the patches).</li>
<li>an edge-map (connections between patches) and,</li>
<li>a vertex-map (centroid representations of the patches).</li>

Comment on lines +89 to +91
In addition, <em>r.connectivity.distance</em> outputs a cost distance
raster map for every input area which later on are used in
<em>r.connectivity.corridors</em> (together with output from
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In addition, <em>r.connectivity.distance</em> outputs a cost distance
raster map for every input area which later on are used in
<em>r.connectivity.corridors</em> (together with output from
In addition, <em>r.connectivity.distance</em> outputs a cost distance
raster map for every input area. These are used later on in
<em>r.connectivity.corridors</em> (together with the output of

@ecodiv
Copy link
Contributor Author

ecodiv commented Jun 11, 2024

I plan to post a fix for v.db.join tonight and then create a PR for the new r.connectivity.distance

@ninsbl how far are you with this? If you need somebody to test things, let me know.

ninsbl added a commit to ninsbl/grass-addons that referenced this pull request Jun 18, 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.

3 participants