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

Query by Sets #480

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Query by Sets #480

wants to merge 21 commits into from

Conversation

JakeWags
Copy link
Member

@JakeWags JakeWags commented Feb 12, 2025

Does this PR close any open issues?

Closes #375

Give a longer description of what this PR addresses and why it's needed

This pull request introduces several updates to the UpsetConfig and related components, primarily focusing on adding support for set queries and updating configuration versions. The key changes include the addition of new types, conversion functions, and utility methods to handle set queries, as well as updates to existing functions to accommodate these changes.

For query by sets, one of the differences between the "old" upset behavior and this is that the query now removes all elements OTHER than those that are included in the query. This will override any aggregation, but keep any sorting and filtering within the query.

Note: I personally am not a fan of the alert for setting the name, this is just a simple solution. A textbook may be difficult to implement within the SVG, but I haven't tried.

Configuration and Type Updates:

  • Added new types SetQuery, SetQueryMembership, and Version0_1_2 to support set queries and updated configurations. (packages/core/src/types.ts) [1] [2] [3] [4]
  • Updated UpsetConfig version to 0.1.2 and included the setQuery field. (packages/core/src/types.ts, packages/core/src/defaultConfig.ts) [1] [2]

Conversion Functions:

  • Added conversion functions to transition configurations from version 0.1.0 to 0.1.1 and from 0.1.1 to 0.1.2. (packages/core/src/convertConfig.ts) [1] [2]

Rendering and Query Handling:

  • Introduced new utility functions flattenRows and getQueryResult to handle set queries and flatten hierarchical row structures. (packages/core/src/render.ts) [1] [2]
  • Updated existing functions sortByRR, filterRR, and getRows to incorporate set query handling. (packages/core/src/render.ts) [1] [2]

Type Checking:

  • Added type-checking functions for SetQuery, SetQueryMembership, and SetMembershipStatus. (packages/core/src/typecheck.ts)
  • Updated isUpsetConfig to validate the new setQuery field and version 0.1.2. (packages/core/src/typecheck.ts) [1] [2] [3] [4]

Utility Methods:

  • Added isPopulatedSetQuery to check if a SetQuery is populated. (packages/core/src/typeutils.ts)

These changes enhance the flexibility and functionality of the UpsetConfig by allowing more complex queries and ensuring compatibility with future versions.

Provide pictures/videos of the behavior before and after these changes (optional)

upset-query-by-sets

Have you added or updated relevant tests?

  • Yes
  • No changes are needed
  • not yet!

Have you added or updated relevant documentation?

  • Yes
  • No changes are needed

Are there any additional TODOs before this PR is ready to go?

TODOs:

  • Write automated playwright tests
  • Fix sizebar scaling with query row

Copy link

netlify bot commented Feb 12, 2025

Deploy Preview for upset2 ready!

Name Link
🔨 Latest commit cfbea7b
🔍 Latest deploy log https://app.netlify.com/sites/upset2/deploys/67b7a6189a004d0008c5bb5a
😎 Deploy Preview https://deploy-preview-480--upset2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@NateLanza NateLanza left a comment

Choose a reason for hiding this comment

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

Changes needed; see comments. I think you might want to check that eslint is working for you as some of these should have been caught by your linter/typechecker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query by sets
2 participants