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

Add post enumeration modification to generate.ts #187

Merged
merged 28 commits into from
Aug 8, 2016

Conversation

espressoroaster
Copy link
Member

@espressoroaster espressoroaster commented Aug 7, 2016

fix #176

- add nominalScaleForHighCardinality config
- add smallBandSizeForFacet config
- add smallBandSizeForHighCardinality config
@espressoroaster espressoroaster force-pushed the fo/168-channel-cardinality-constraint branch from 82edc38 to d460213 Compare August 7, 2016 19:51
@@ -150,6 +156,11 @@ export const DEFAULT_QUERY_CONFIG: QueryConfig = {
preferredNominalAxis: Channel.Y, // nominal on y makes it easier to read.
preferredFacet: Channel.ROW, // row make it easier to scroll than column

// what should their default values be?
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you look at VLUI?

@espressoroaster espressoroaster changed the title [WIP] add post enumeration modification to generate.ts add post enumeration modification to generate.ts Aug 8, 2016
@@ -96,6 +96,12 @@ export interface QueryConfig {
// Effectiveness Preference
maxGoodCardinalityForColor?: number; // FIXME: revise
maxGoodCardinalityForFacet?: number; // FIXME: revise

// POST ENUMERATION MODIFICATION
Copy link
Member

Choose a reason for hiding this comment

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

We call it stylize - this is inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Move this above

// Effectiveness Preference

and change Effectiveness Preference to ALL CAPS.

@kanitw
Copy link
Member

kanitw commented Aug 8, 2016

  • combine the high cardinality and facet one

@espressoroaster espressoroaster changed the title add post enumeration modification to generate.ts [WIP] add post enumeration modification to generate.ts Aug 8, 2016
let specM = SpecQueryModel.build({
mark: Mark.BAR,
encodings: [
{channel: Channel.Y, field: 'Q', scale: {name: 'scale', values: [true, false]}, type: Type.QUANTITATIVE}
Copy link
Member

Choose a reason for hiding this comment

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

Q bandSize?????

@espressoroaster espressoroaster changed the title add post enumeration modification to generate.ts [WIP] add post enumeration modification to generate.ts Aug 8, 2016

describe('stylize', () => {
describe('smallBandSizeForHighCardinalityOrFacet', () => {
it('should assign a bandSize of 12 if cardinality of Y is over 10 and bandSize is not already set', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Don't you have to add one case when it's already set?

@espressoroaster espressoroaster changed the title [WIP] add post enumeration modification to generate.ts add post enumeration modification to generate.ts Aug 8, 2016
encQIndex[Channel.COLOR] = specM.getEncodingQueryByChannel(Channel.COLOR);

const colorEncQ = encQIndex[Channel.COLOR];
if ((colorEncQ !== undefined) && (colorEncQ.type === Type.NOMINAL) &&
Copy link
Member

Choose a reason for hiding this comment

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

why 10? Use value in the config.

@kanitw kanitw changed the title add post enumeration modification to generate.ts Add post enumeration modification to generate.ts Aug 8, 2016
@kanitw kanitw merged commit 2f68187 into master Aug 8, 2016
@kanitw kanitw deleted the fo/168-channel-cardinality-constraint branch August 8, 2016 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants