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

[FEATURE] [needs-docs] classifyAroundZero #5161

Closed
wants to merge 3 commits into from

Conversation

pierreloicq
Copy link
Contributor

@pierreloicq pierreloicq commented Sep 9, 2017

Description

I added a new method of classification in the graduated methods. It is aimed at plotting data that are centered on zero, with a class around zero (like [-1,1] instead of [-1,0],[0,1] ), and with a colorbar that stay symmetric around zero. It uses the prettyBreaks method. If data are all positive or negative, an alert box appear.

It didn't pass every test (I guess it's normal since some test not passed were very far from what I modified)
It passed the test qgis_graduatedsymbolrenderertest. I didn't add anything in this test.

aroundzero2

…made to plot data around zero with a colorbar that stay centered and symmetric on zero. Use the prettyBreaks method.
@nyalldawson
Copy link
Collaborator

@anitagraser you might be best placed to review the classification logic used here, want to take a look?

@anitagraser
Copy link
Member

The name of the new mode should indicate that prettyBreaks is used. Otherwise users won't know.

I'm not sure a warning is needed when all values are positive or negative. Why not fall back to regular prettyBreaks instead?

Could 'symmetric around zero' be an option for all modes? Then it could be a checkbox.

I don't have a build environment set up, so I cannot comment on whether it actually works ok.

@pierreloicq
Copy link
Contributor Author

Hello,
Yes I thought about this possibility to do it for every method (here) but I think it does not make so much sense (will someone ever use that ?) and it's more complex, especially for the Ward method.
I chose to make a new method instead of a ticking box for visibility reason, but I can try to do a ticking box instead.

@nyalldawson
Copy link
Collaborator

What I'm interested to know is whether there's any "standard"/"scientifically accepted" techniques used for classification in this situation. @anitagraser are you aware of any?

I'd like to make sure we're not implementing our own algorithm here when there's a standard one we should be using instead.

@anitagraser
Copy link
Member

I'd have to check what the cartography literature has to say about this.

@anitagraser
Copy link
Member

I think a generic design will be able to support more use cases. One approach would be a checkbox "symmetric around" followed by a numeric input field with default value 0.0 because, as @cartocalypse pointed out correctly on Twitter, other values can be "the center" as well.

@pierreloicq
Copy link
Contributor Author

My check box is ready !
Hum...looks like it will not be useful, because if we want to propose the numeric input, it may be a float (not a pretty value) and then the pretty breaks will not be adapted.
I guess in that case the size of each class should be a user choice as well and replace the number of classes choice. So I go back to a new method in the classification menu (like on the 1st screenshot) ?

prettybreaksaroundzero

@anitagraser
Copy link
Member

if we want to propose the numeric input, it may be a float (not a pretty value) and then the pretty breaks will not be adapted.

I'm not sure I follow / see the problem. For pretty breaks, the input could be restricted to only allow whole numbers.

A "stupid" classifier, where the user specifies the class size, would also be useful sometimes but that's a different discussion in my opinion.

@pierreloicq
Copy link
Contributor Author

I guess you mean "For pretty breaks, the input could be restricted to only allow values determined as breaks/pretty". That seems fair.

@pierreloicq
Copy link
Contributor Author

Hello,
Do you know if my PL has to be accepted before the 15 or if I am allowed to finish later?
Thank you

@Gustry
Copy link
Contributor

Gustry commented Sep 12, 2017

Feature freeze is end of October for QGis 3: https://www.qgis.org/en/site/getinvolved/development/roadmap.html

So you still have time

@andreasneumann
Copy link
Member

Hi @pierreloicq - thanks for looking into this issue!

I think we are talking about bi-polar color ramps here. But I don't think that we should introduce a new classification method for this special case.

As to my understanding, this is trying to make sure that classes are symmetric around a certain point (be it 0, 1, 100). I agree with @cartocalypse (Twitter) that we should not restrict this to zero only. There are valid use cases to have the symmetry around 1 or 100 or other values.

This method would have to be restricted to "odd" numbers of classes - right? "even" numbers wouldn't result in a symmetric distribution around a center class - right? Or how would this work with f.e. six classes?

Also, couldn't this "symmetry" be implemented for all the other classification methods as well, or what is so special about pretty breaks here?

@pierreloicq
Copy link
Contributor Author

Hello,
For the moment, the idea is to propose to user to delete one of the breaks that is given by the pretty breaks method. Since it's the pretty, I am sure that the next and previous break are at the same distance from the symmetry point and they will form a class around it. Number of class is adapted by removing the break that are further than the farthest break of the other side (except the final one). So I guess it always provide an odd number of class, which does not always equate the input number of classes.

The symmetry may be applied on all the methods that have a constant break distance (equal interval, s.d. method and pretty breaks).
Lets try for equal interval :
Let's say min = -2.3 and max = 3.6 and I want a symmetry at 1.0 and 5 classes (if 4 is input we take 3 ou 5).
-2.3 is at 3.3 from 1 and 3.6 is at 2.6 from 1. I take the maximum (3.3). (taking 2.6 is also a possibility)
So (2*3.3)/5 = 1.32 is the size of each class.

You see here that methods will be different for each methods. For the prettyBreaks method, the symmetry point can only be a pre-existing break and the class located on the symmetry point is 2x larger than the others.
For the equal interval method, you are obliged to propose any float and the classes can all be of same size.
For the s.d. method, we only need to put the chosen symmetric point instead of the median.

I don't see how we could apply it to the Jenks and quantile method since distance between breaks is not constant by definition.

@kannes
Copy link
Contributor

kannes commented Sep 21, 2017

@cartocalypse here :)

First of all, this is a feature I long wished for so that you very very much for working on it! I meant to jump into this discussion earlier, sorry for forgetting.

I agree that this would be best as additional feature, not a new method. For the standard deviation classification method I think it should only be allowed with the mean as pivot, otherwise it makes no sense (to my understanding).

To me there are three options that one needs to choose:

a) Symmetric around:

  • zero
  • average/mean
  • median
  • user-specified value (maybe with expression support?)

b) Number of classes to each side

c) The kind of method to use

So how about having a new widget in the same line as the Mode and Classes widgets that gets enabled on supported modes and let's the user toggle it and set the pivot? If used, restrict the number of classes to multiples of 2.

For potential future work I would like to draw attention to the histogram, where mean and stdev are displayed. It would be fantastic if one could set the pivot there as a special class break.

Again, this is a great feature, it will make working with QGIS much easier!

@pierreloicq
Copy link
Contributor Author

Hello,
I created a new branch and hence a new PR here: #5370
Thank you

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.

6 participants