-
Notifications
You must be signed in to change notification settings - Fork 0
docs: 📝 add design docs for the request file #65
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
base: main
Are you sure you want to change the base?
Changes from all commits
8667f4d
20e3007
458981d
10df2b0
c66421a
aa3bbe0
cda4e48
9adf245
013e59a
2e93449
cdc524a
9347453
b3da1c0
aae4b26
da8fa3f
e613d93
9fe1cc3
f0bdb36
9d43ec6
eda9c81
ac34861
2ecec38
f6941a6
d007625
ff41b61
c4df516
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| [files] | ||
| extend-exclude = ["*.html", "*.css", ".quarto/*", "_site/*", "*.svg"] | ||
| extend-exclude = ["*.html", "*.css", ".quarto/*", "_site/*", "*.svg", "*.js"] | ||
|
|
||
| [default.extend-words] | ||
| ratatui = "ratatui" |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -170,11 +170,29 @@ flowchart LR | |||||
| ## {{< var planned >}} `seedcase-propagate check` | ||||||
|
|
||||||
| The `check` command confirms that the `request.yaml` file is correctly | ||||||
| structured and that it matches the content of the source metadata file. While | ||||||
| internally `build`, `subset`, and `create-request` run this check when | ||||||
| processing or creating the `request.yaml` file, we also expose it so the user | ||||||
| can manually check the request file (e.g. if included within an automated | ||||||
| pipeline). It has the following signature with two positional arguments: | ||||||
| structured and that it matches the content of the source metadata file. More | ||||||
| precisely, the following checks are made: | ||||||
|
|
||||||
| - Required sections and keys are present. | ||||||
| - The referenced data package has the same id and name as the source metadata | ||||||
| file. | ||||||
| - The referenced resources exist in the data package. | ||||||
| - The requested columns exist in the selected resources. | ||||||
| - Row filters reference existing columns. | ||||||
| - Row filters use supported SQL-like syntax. | ||||||
| - Only allowed SQL operations and shorthands are used. | ||||||
|
|
||||||
| While most of these check are likely easier to test directly in the YAML file, | ||||||
| the subset queries filters can also be converted to SQL and checked with | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| existing SQL parsers or checkers where possible. This avoids implementing a | ||||||
| custom expression parser for the request format. | ||||||
|
|
||||||
| Internally `build`, `subset`, and `create-request` run these checks when | ||||||
| processing or creating the `request.yaml` file. We also expose it as a command | ||||||
| so the user can check the request file independently (e.g. if included within an | ||||||
| automated pipeline). | ||||||
|
|
||||||
| `check` has the following signature with two positional arguments: | ||||||
|
|
||||||
| ```bash {filename="Terminal"} | ||||||
| seedcase-propagate check [REQUEST] [SOURCE] | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,312 @@ | ||||||||||
| --- | ||||||||||
| title: "Request file" | ||||||||||
| --- | ||||||||||
|
|
||||||||||
| The request file, `request.yaml`, contains the instructions for requesting a | ||||||||||
| subset of data from a data package. The file is created by a Requester through | ||||||||||
| the web application, terminal interface, or even manually (though not | ||||||||||
| recommended). The file is used by an Owner to check and generate the requested | ||||||||||
| data subset. You can | ||||||||||
| [read more about the Requester and Owner user types in the Design overview page](/docs/design/interface/inputs.qmd) | ||||||||||
|
|
||||||||||
| The request file is intended to be both human-readable and machine-readable. It | ||||||||||
| should be simple enough for a Requester to understand and, when needed, edit | ||||||||||
| manually. At the same time, it should be structured enough that Propagate can | ||||||||||
| check it against the data package metadata and convert it into an SQL query for | ||||||||||
| generating the requested data subset. | ||||||||||
|
|
||||||||||
| ## Format | ||||||||||
|
|
||||||||||
| The request file uses YAML. YAML was chosen because it is more readable for | ||||||||||
| manual editing than JSON and less verbose than TOML for lists of columns and row | ||||||||||
| filters. | ||||||||||
|
|
||||||||||
| ## Structure | ||||||||||
|
|
||||||||||
| The request file contains the following top-level sections: | ||||||||||
|
|
||||||||||
| - `request`: Information about this specific request. | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| - `requester`: Information about the person requesting the data. | ||||||||||
| - `project`: Information about the research project that requires the data. | ||||||||||
| - `data-package`: Information about the data package the request applies to. | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| - `subsets`: The requested data subsets, with one item per resource. | ||||||||||
|
|
||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| Each of these is elaborated on below: | ||||||||||
|
|
||||||||||
| ### Request | ||||||||||
|
|
||||||||||
| The `request` includes information about the request itself, such as when and | ||||||||||
| why it was created. This helps an Owner organize and track requests, which is | ||||||||||
| especially beneficial for data packages that receive large volumes of requests. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| request: | ||||||||||
| date-modified: DATE | ||||||||||
| date-created: DATE | ||||||||||
|
Comment on lines
+44
to
+45
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| motivation: TEXT | ||||||||||
|
lwjohnst86 marked this conversation as resolved.
|
||||||||||
| iteration: NUMBER | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| - `date-modified`: When the request was last modified. This can act as an ID | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| which facilitates referring to a specific iteration of a request, e.g. in | ||||||||||
| dialog between the Owner and Requester. Precision to the second should be | ||||||||||
| sufficient. | ||||||||||
| - Note that this field is automatically populated when the request is saved | ||||||||||
| via a user interface (be it the web app or the TUI). If the request is | ||||||||||
| edited manually, it must be updated manually. | ||||||||||
| - The format should be ISO 8601, e.g. `2026-07-06T01:45:34-12:00` | ||||||||||
|
lwjohnst86 marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| <!-- TODO this is this the most suitable date format? It seems like it makes sense to include timezone info so it can be easily interpreted by both sides if they are in different timezones --> | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Rarely is timezone information helpful, it often makes things more confusing (in terms of processing it). I think date at UTC is enough for what Propagate aims to do.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, what's the motivation for why timezone info rarely would be useful? A use case could be someone opening up the file and seeing a different time than when they created it because they are not in the UTC. I think that including timezone info makes it explicit what is being recorded and it's more inclusive to people working with propagate in different timezones. It could also help when owners and requesters are from different time zones. What's a use case when not having timezone info would be helpful so that we can think about what's more relevant to propagate?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Working with datetimes is difficult enough, but adding in timezone is brutal. See this best practice answer here: https://stackoverflow.com/questions/44965545/best-practices-with-saving-datetime-timezone-info-in-database-when-data-is-dep Basically, store the current time but set it as UTC and if necessary use timezone for display purposes only. For our cases, we aren't really displaying anything so it isn't really an issue. And the datetime fields are only for tracking and identification purposes, not for any interesting/complicated analyses. Also, for propagates case, it is very very unlikely that requesters are outside of the owners timezone. Most of the time it is people within the same group or institution. VERY rarely is it someone outside of that. Trust me here, we want to avoid this absolute headache that is timezones and datetimes. Especially as timezone information is not relevant for Propagate, at all. |
||||||||||
| - `date-created`: When the first iteration of the request was created. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Waiting on resolving the discussing above before making changes here |
||||||||||
| <!-- TODO The creation date would only be correctly populated if the requester uploads their old request in the interface when they need to make a modification. If they rewrite the request from scratch, they could (mistakenly) put an incorrect date here, but that's fine I think. --> | ||||||||||
|
signekb marked this conversation as resolved.
|
||||||||||
| - `motivation`: Explanation for why the requester's project needs access to this | ||||||||||
| data subset. This is used by the Owner to check whether the requested data is | ||||||||||
| relevant to the project. The motivation is particularly needed for human | ||||||||||
| health data since there are legal requirements (e.g. GDPR) to declare for what | ||||||||||
| purposes the data is used. | ||||||||||
| - `iteration`: The iteration of requests from this project for this data | ||||||||||
| package. | ||||||||||
| <!-- TODO This is a "maybe" since it seems a bit less reliable. It could be automatically incremented if a file is loaded into the web-interface, but there is not way to know if that file was a saved version that was never sent (ie the iteration should not be updated) or if it was sent and is now being modified for sending again (ie the iteration should be updated). It could be manually filled in of course, but we should think about who this is mainly useful for; if it is the owner then they could run something on their end to find out how many requests they have; if it is someone viewing the data package then it's more useful to try to have it in the file itself. --> | ||||||||||
|
|
||||||||||
| ### Requester | ||||||||||
|
|
||||||||||
| The `requester` section describes who is requesting the data. This person | ||||||||||
| doesn't strictly have to be the one working with the data, but they should serve | ||||||||||
| as the main point of contact for the Owner. | ||||||||||
|
|
||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| requester: | ||||||||||
| name: TEXT | ||||||||||
| email: TEXT | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| - `name`: The name of the requester. | ||||||||||
| - `email`: The email of the requester. | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
|
|
||||||||||
| ### Project | ||||||||||
|
|
||||||||||
| The `project` section contains information about the research project that | ||||||||||
| requires the data. A project is defined as a body of work that result in a | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| research output, e.g. a paper, report, website, dashboard, or analysis. This | ||||||||||
| could be a part of a PhD student's thesis or a collaboration research project | ||||||||||
| for a group of researchers. This field is necessary to ensure that the data | ||||||||||
| requested is aligned with what it is intended to be used for, which can a legal | ||||||||||
|
|
||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| requirement (e.g. in GDPR). | ||||||||||
|
|
||||||||||
| - `name`: A short machine-readable project name, using lowercase letters and | ||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think more about this, I wonder if
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this depends on whether we want the name to be something shorter than the title and not just a kebab case version of it - which I think we do. E.g., in our example data package
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you agree, it might be helpful if the examples in this list were updated to reflect this difference 🌻
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good consideration! I think it depends on what this name field is meant to be used for. We previously discussed that this should be machine readable and used for identification purposes. That's quite different from the description of the However, if it is meant for human consumption, then I agree with you that it would benefit from being shorter than the title. I think either works and I also think we don't need
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||
| hyphens where possible, e.g. `my-research-project`. This is mainly used for | ||||||||||
| identification purposes when managing several requests. | ||||||||||
| - `title`: A longer human-readable project title, e.g. `My research project`. | ||||||||||
| This could be used by the Owner to publicly display the data package requests. | ||||||||||
| - `description`: A summary of the project, including details like | ||||||||||
| background, rationale, basic methods, and potential research questions. This | ||||||||||
| information, combined with the `motivation`, is mainly to help the Owner | ||||||||||
| ensure that the data requested matches what it will be used for. | ||||||||||
|
|
||||||||||
| ### Data package | ||||||||||
|
|
||||||||||
| The `data-package` section identifies the data package the request applies to. | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| This information will be automatically populated from the data package metadata | ||||||||||
| when creating a request via the web application or the TUI. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| data-package: | ||||||||||
| id: TEXT | ||||||||||
| name: TEXT | ||||||||||
| version: X.Y.Z | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| - `id`: A unique identifier of the data package. | ||||||||||
| - `name`: A human readable name to refer to. It is lowercased and doesn't | ||||||||||
| include any spaces. E.g., `my-data-package`. | ||||||||||
| - `version`: Helps the Owner detect which version of the data package the | ||||||||||
| request is for. | ||||||||||
|
|
||||||||||
| ### Subsets | ||||||||||
|
|
||||||||||
| The `subsets` section describes which data subsets the Requester would like to create. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| subsets: | ||||||||||
| - resource: RESOURCE-NAME | ||||||||||
| columns: | ||||||||||
| - COLUMN-NAME | ||||||||||
| - COLUMN-NAME | ||||||||||
| - COLUMN-NAME | ||||||||||
| rows: | ||||||||||
| - "ROW-CONDITION" # As SQL | ||||||||||
| # Alternatively, this more structured format, see comparison below | ||||||||||
| - column: COLUMN-NAME | ||||||||||
| operator: "OPERATOR" # E.g. ">" | ||||||||||
| value: VALUE-TO-COMPARE-AGAINST | ||||||||||
| - resource: RESOURCE-NAME | ||||||||||
| columns: | ||||||||||
| - COLUMN-NAME | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| <!-- TODO When editing the file directly, do we require the user to enter row conditions (and maybe all other values too) as strings? It seems like we have to in order not to have yaml parsing potentially break if they do something illegal. --> | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I think that's a good plan 👍
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which one, just the row conditions or all values? And should we always do it; ie even when the file is populated from the web interface (it seems like that would be helpful to always encourage it, e.g. if someone goes in to make an edit manually to a file created from the UI)?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I think we should always parse as string, to keep it simpler. When we convert into SQL/running the subset, we can do the corrections there to the right types. |
||||||||||
|
|
||||||||||
| - `resource`: Specifies the data resource to create the subset from. Multiple | ||||||||||
| subsets can be specified in the same request by repeating the `resource` key. | ||||||||||
| This can be done either with the same resource multiple times or with | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| different resources for each. | ||||||||||
|
Comment on lines
+152
to
+153
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for my understanding: In what scenario does it make sense to include the same resource multiple time?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is still up for discussion whether we prefer to support that or not. I mentioned the following in #65 (comment), but that comment is not easy to find now:
As a direct answer to your question; I would imagine a scenario where someone is interested in looking into column A for a subset of the population and then column B for another subset. If the data was very large or a column very heavy (e.g. images or geo-coordinates), it could be impractical if we forced the user to pull column A and B for the union of two subset (e.g. they needed to pull down images for 10 rows as an example and we force them to pull 1 million images because that's how big the subset for the other column is).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not completely sure where this belongs, maybe in this file, but probably should be clarified in the requirements. For GDPR purposes (and other legal/privacy regulations), as well as for statisticaly reasons, any subsetting by rows in any resource will lead to the observational units (participants if human) being the same across all resources. For example, if in So, in your example, that won't be allowed to request from the same resource more than once. But actually, your example highlights that they won't be given everything when they only need a specific amount (e.g. the ask for 10 rows and only get images for those 10 rows). |
||||||||||
| - `columns`: The columns to include. Each list item must correspond to an exact | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, we do not include a shorthand for saying "include all columns from this resource"?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lwjohnst86 mentioned that he prefers if all columns that are part of the data subset are mentioned explicitly in the file (which I think is part of why we also don't have exclusions, ranges, etc). I believe the reasoning is that a person should be able to see which variables are used in the project when opening
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it could be that if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that was one of the suggestions in my previous comment in the file, so that there would be symmetry between the meaning of an empty value for I thought you wanted all columns listed explicitly for the reasons I mentioned above, but if that is no longer the case, then we just need to decide whether it is clear enough that a request that looks like this, requests all the columns and rows from subsets:
- resource: resource1
- resource: resource2
columns:
- col1
rows:
- col1 > 50I think it probably is clear enough, and we can write that "...if you don't want to request all the data in a resource, you can select specific columns and rows to include in the data subset..."
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really like the idea of there being symmetry between empty values from
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think with @martonvago comment about mirroring the web app/tui interface and how it would actually be implememted, it makes sense that no columns selected means all columns requested from the resource (which maybe? mimics how we would do it in the app..?). So yea, let's do that, as @signekb says, it also has some symmetry. |
||||||||||
| column name. SQL-like shorthands are not supported in order to keep the | ||||||||||
| request file simple and to avoid parsing unsafe queries of the data (e.g. via | ||||||||||
| SQL injection). This can not be left empty / unspecified; at least one column | ||||||||||
| must be included in the `columns` field. | ||||||||||
| - `rows`: Describes which rows to include in the subset. Can reference columns | ||||||||||
| that are not a part of the subset. If the `rows` key is empty or not included, | ||||||||||
| all rows will be selected. | ||||||||||
|
|
||||||||||
| There are currently two candidates for what this might look like, see below. | ||||||||||
|
|
||||||||||
| #### Row condition alternatives | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good discussion about this tricky bit! My two cents: filters:
growth_records:
and:
- is_null:
column: location_country
not: true
- or:
- equal:
column: growth_stage
value: flowering
- greater_than:
column: observation_date
value: 2020-01-01Or the more verbose: filters:
growth_records:
operator: and
children:
- column: location_country
operator: is null
not: true
- operator: or
children:
- column: growth_stage
operator: "="
value: flowering
- column: observation_date
operator: ">"
value: 2020-01-01This structure is a modified version of your alternative 2 that also includes grouping column conditions. The naming and exact structure can be refined of course. I like this because keeping the structure aligned with the interface allows complexity (creating the SQL query) to be pushed further down into the core bits of Propagate. This strikes me as both safer and simpler. Safer because, this way, the user cannot introduce arbitrary SQL-like expressions through the request file. Of course, we would defend against this in alternative 1 by parsing and checking the condition, but that is still riskier than sidestepping the issue altogether. Simpler because we go
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, great ideas/thoughts here! My brain comprehends the second, more verbose option here, but I can see how either could work 🤔
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I also like this. I also prefer the second one because it is more explicit. We could use words like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also in favor of keeping the yaml file structure as close as possible to how information is represented in the graphical UI! Agree with you that it becomes both safer and simpler for us, including simpler to check that only the allowed subset of SQL that we want to support is used. I think the two suggestion above are quite similar to some of the expanded syntax suggestions for alternative two from this issue comment #3 (comment), which I think would become the following for the example above: rows/filters:
- resource: growth_records
all:
- column: location_country
operator: is null
not: true
- any:
- column: growth_stage
operator: "="
value: flowering
- column: observation_date
operator: ">"
value: 2020-01-01I'm happy with all three options we have here in this discussion, and we essentially need to decide what level of verbosity we want. I think the suggestion I have from previously is somewhere between the two above and I have a preference for one of the two more verbose options we have in this discussion. I can comment on a few things in terms of why I picked them, but I'm open to landing on any of the above suggestions (or a combination of them):
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I missed the second half of the comments on #3, sorry! I personally don't have a strong preference between what you suggested there and the examples I put here. So on all points above I'm very happy to go with your preference. I'm also leaning towards
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with your suggestions @joelostblom. These types of comments should be put into the doc (they are design decisions). Can you modify and add it to it? As for 6. technically they are row "keeps", as the condition keeps rows, it doesn't filter them (filter means remove, not keep). Just a small semantic comment 😛
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small - maybe misplaced - comment: "filtering" is both about keeping something and removing something else, so it's ambiguous in that way. |
||||||||||
|
|
||||||||||
| ##### Alternative 1 | ||||||||||
|
|
||||||||||
| - A list of SQL filter conditions that describe which rows to include in the | ||||||||||
| subset. | ||||||||||
| - Multiple row filters for the same resource are combined with `AND` logic. For | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| example, writing | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| rows: | ||||||||||
| - "age > 50" | ||||||||||
| - "sex = 'Man'" | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| is equivalent to writing | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| rows: | ||||||||||
| - "age > 50 AND sex = 'Man'" | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ::: callout-note | ||||||||||
| Since YAML has special syntax for some characters, SQL-like row filters should | ||||||||||
| be quoted when written manually. This avoids ambiguity around characters such as | ||||||||||
| `>`, `:`, `#`, `*`, `&`, and `!`. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| rows: | ||||||||||
| - "col1 > 50" | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| Instead of: | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| rows: | ||||||||||
| - col1 > 50 | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| The unquoted form will often parse correctly, but the quoted form is safer and | ||||||||||
| should be used in documentation and generated files. | ||||||||||
| ::: | ||||||||||
|
|
||||||||||
| - The initial supported row filter syntax includes the following operators: | ||||||||||
| - Equality: `=` | ||||||||||
| - E.g. "age = 18" | ||||||||||
| - Comparisons: `>`, `>=`, `<`, `<=` | ||||||||||
| - E.g. `"age >= 18"` | ||||||||||
| - Membership: `IN` | ||||||||||
| - E.g. `"country IN ('Denmark', 'Sweden')"` | ||||||||||
| - Ranges: `BETWEEN` | ||||||||||
| - E.g. `"visit_date BETWEEN '2020-01-01' AND '2020-12-31'"` | ||||||||||
| - Missing values: `IS NULL`, `IS NOT NULL` | ||||||||||
| - E.g. `"end_date IS NULL"` | ||||||||||
| - Negation: `NOT` | ||||||||||
| - E.g. `NOT =`, `NOT IN`, `NOT BETWEEN` | ||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One slightly annoying thing is the Also it does seem like the most common way to compare inequality for numericals are |
||||||||||
| - This alternative is more closely mapped to how the SQL syntax will look, and | ||||||||||
| the options from the interface would need to be combined into the SQL row | ||||||||||
| filter syntax. | ||||||||||
| - An advantage of this syntax is that it is more flexible. This might mainly | ||||||||||
| matter when editing manually as we allow more expressiveness here. | ||||||||||
| - A drawback could be that requesters write things we don't want to allow and | ||||||||||
| that we would need to filter out at the SQL level or by parsing the string | ||||||||||
| which is likely more error prone than a more structured format. | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
|
|
||||||||||
| #### Alternative 2 | ||||||||||
|
|
||||||||||
| - A more structured key-value mapping for the different components of the filter | ||||||||||
| condition. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| rows: | ||||||||||
| - column: col1 | ||||||||||
| operator: ">" | ||||||||||
| value: 50 | ||||||||||
|
joelostblom marked this conversation as resolved.
|
||||||||||
| - column: col2 | ||||||||||
| operator: "in" | ||||||||||
| value: | ||||||||||
| - dog | ||||||||||
| - cat | ||||||||||
| - column: col3 | ||||||||||
| operator: between | ||||||||||
| value: | ||||||||||
| - 2012-01-01 | ||||||||||
| - 2012-12-31 | ||||||||||
|
Comment on lines
+248
to
+249
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is tricky, as it would require that we sort or something after reading the values.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't trust the order, we could name the start and end points
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, in #3 (comment) I suggested to use One more thought is that we can get rid of between altogether and replace it with two conditions, eg. instead of the example above, we could require the following: - column: col3
operator: "<="
value: 2012-01-01
- column: col3
operator: ">="
value: 2012-12-31I don't think that would be an inconvenience when specified in a UI. It also makes it more explicit whether the end points are inclusive or not, which I don't think is clear to someone using BETWEEN unless they have previous SQL knowledge. |
||||||||||
| ``` | ||||||||||
|
|
||||||||||
| - This alternative is more closely mapped to the interface, e.g. the dropdown | ||||||||||
| for the operator corresponds to the "operator" key here and the condition | ||||||||||
| groups in the interface are also more similar to how they are grouped here | ||||||||||
| with "any". | ||||||||||
|
signekb marked this conversation as resolved.
|
||||||||||
| - An advantage of this syntax is that it is more structured, so it would be | ||||||||||
| easier to check that e.g. only allowed operators are used if someone manually | ||||||||||
| edits the file. | ||||||||||
| - If we want to allow more flexibility when the file is edited manually, it | ||||||||||
| could be harder with this more strict spec, but it doesn't sound like that is | ||||||||||
|
signekb marked this conversation as resolved.
|
||||||||||
| our intended use case currently. | ||||||||||
|
|
||||||||||
| ### A complete example | ||||||||||
|
|
||||||||||
| A complete request file might look like this. | ||||||||||
|
|
||||||||||
| ```{.yaml} | ||||||||||
| request: | ||||||||||
| date-modified: 2026-07-06T01:45:34-12:00 | ||||||||||
| motivation: | | ||||||||||
| We would like access to metabolic and block variables to evaluate | ||||||||||
| our hypothesis regarding ... | ||||||||||
| date-created: 2026-07-04T01:44:34-12:00 | ||||||||||
| iteration: 2 | ||||||||||
|
|
||||||||||
| project: | ||||||||||
| name: | ||||||||||
| title: Metabolic costs | ||||||||||
| description: | | ||||||||||
| Our project investigates the gas exchange during metabolism | ||||||||||
| with the aim to determine ... | ||||||||||
|
|
||||||||||
| data-package: | ||||||||||
| id: 8dbef2a2-0354-4996-bc9e-71a6c79aec7d | ||||||||||
| name: example-seed-beetle | ||||||||||
| version: 0.5.1 | ||||||||||
|
|
||||||||||
| subsets: | ||||||||||
| # TODO Showing both row syntaxes for now | ||||||||||
| - resource: metabolic-rate | ||||||||||
| columns: | ||||||||||
| - o2_consumed | ||||||||||
| - co2_produced | ||||||||||
| rows: | ||||||||||
| - "o2_consumed > 50" | ||||||||||
| - resource: metabolic-rate | ||||||||||
| columns: | ||||||||||
| - strain | ||||||||||
| - activity | ||||||||||
| rows: | ||||||||||
| - column: block | ||||||||||
| operator: "=" | ||||||||||
| value: block1 | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Non-goals | ||||||||||
|
|
||||||||||
| Propagate is not intended to be a general-purpose query engine. The initial | ||||||||||
| request format should not support joins, grouping, arbitrary functions, random | ||||||||||
| sampling, or row selection by position. We also don't support shorthands such as `*` since that would make it less | ||||||||||
| explicit which columns are used from just looking at the request file without | ||||||||||
|
Comment on lines
+308
to
+311
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this paragraph should be at the top as a callout block. |
||||||||||
| also seeing the data package metadata. | ||||||||||
Uh oh!
There was an error while loading. Please reload this page.