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 histogram bindings #30033

Merged
merged 2 commits into from Jun 3, 2019
Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented May 30, 2019

Hopefully fixes #29700

@elpaso elpaso added Bug Either a bug report, or a bug fix. Let's hope for the latter! Needs Forwardporting labels May 30, 2019
@@ -378,8 +378,8 @@ class CORE_EXPORT QgsRasterInterface
maximum = PyFloat_AsDouble( a3 );
}

QgsRasterHistogram h = sipCpp->histogram( a0, a1, minimum, maximum, *a4, a5, a6, a7 );
sipRes = &h;
QgsRasterHistogram *h = new QgsRasterHistogram( sipCpp->histogram( a0, a1, minimum, maximum, *a4, a5, a6, a7 ) );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this need wrapping in "sipConvertFromType" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nyalldawson please have a look to my last commit.

@nyalldawson
Copy link
Collaborator

Is it worth adding a python test for this method? Is there an existing one? Gut feeling is that there's likely NO tests for this method, it predates the focus on stability.

@elpaso elpaso merged commit 1fcc982 into qgis:release-3_4 Jun 3, 2019
@elpaso
Copy link
Contributor Author

elpaso commented Jun 3, 2019

Is it worth adding a python test for this method? Is there an existing one? Gut feeling is that there's likely NO tests for this method, it predates the focus on stability.

I'll see what I can do.

elpaso added a commit to elpaso/QGIS that referenced this pull request Jun 9, 2019
Fwd port of PR qgis#30033
Fixes qgis#29700

With a new test for the bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Needs Forwardporting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants