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

Sort category names alphabetically in legends #3218

Merged
merged 4 commits into from
Apr 16, 2015

Conversation

alonsogarciapablo
Copy link
Contributor

Fixes #2808.

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor Author

@javisantana, this seems to get the job done. 👍

Another option would be to sort the categories here.

@javisantana
Copy link
Contributor

i love when things work by chance 🇪🇸

as a side comment, all those functions that we have to get analytics from a table should be in cartodb.js/src/api/sql.js

The reason is that a lot of times the user want to get stats from the table and they need to do their own queries so having:

var sql = cartodb.SQL({ username: 'rambo' });
sql.categoriesForColumn(...., function() {
});

But there is another reason to do that. Postgres has its own statistics for columns in tables, so for example we could have a "fast" path for this using pg_stats which collections information for columns:

# select chr((65 + round(random() * 25))::integer) as ch into table_test from generate_series(1, 100000);
SELECT 100000
# select most_common_vals, most_common_freqs from pg_stats where tablename = 'table_test';
                   most_common_vals                    |                                                                                                                     most_common_freqs
-------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 {X,L,S,Y,P,J,O,F,T,D,N,Q,K,B,W,E,C,U,G,H,M,I,R,V,Z,A} | {0.0433333,0.0414667,0.0410667,0.0409667,0.0408,0.0405667,0.0405667,0.0404667,0.0404667,0.0403333,0.0402333,0.0402333,0.0401667,0.0396667,0.0396667,0.0396333,0.0393333,0.0391667,0.0391333,0.0388667,0.0387667,0.0387333,0.0383,0.0380667,0.0203,0.0197}

cc @CartoDB/frontend

@viddo
Copy link
Contributor

viddo commented Apr 16, 2015

Thx for the info! Although, I'm thinking, apart from informing us in-context here, shouldn't this be documented somewhere?

@xavijam
Copy link
Contributor

xavijam commented Apr 16, 2015

Agree with @viddo.

@xavijam
Copy link
Contributor

xavijam commented Apr 16, 2015

In any case 👍

@javisantana
Copy link
Contributor

@viddo what do you mean with documented?

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@viddo
Copy link
Contributor

viddo commented Apr 16, 2015

..., all those functions that we have to get analytics from a table should be in cartodb.js/src/api/sql.js

I interpret this as there are functions that should be located in the sql.js there are currently located elsewhere (where they shouldn't be). But how would should we know? After reading this issue we will have it in the back of our heads (at best), but what about new people joining the project in the future?

So, what I mean with documented is that this decision/convention/guideline should be written down and explained (like above), e.g. in the wiki or a versioned doc/ dir.

- Refactored how we limit the number of categories displayed in the legend.
- Removed code that transforms colors to categories.
@alonsogarciapablo
Copy link
Contributor Author

@javisantana PTAL at this final commit.

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@javisantana
Copy link
Contributor

🇪🇸

alonsogarciapablo added a commit that referenced this pull request Apr 16, 2015
Sort category names alphabetically in legends
@alonsogarciapablo alonsogarciapablo merged commit 9bfc934 into master Apr 16, 2015
@alonsogarciapablo alonsogarciapablo deleted the 2808-order-legend-items branch April 16, 2015 12:39
@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@andy-esch
Copy link
Contributor

thanks for the sorting, you guys are superstars

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.

alphabetically ordered legend items
6 participants