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

make subject_data more consistent across aggregated and non-aggregated mode #215

Closed
surchs opened this issue Nov 8, 2023 · 1 comment
Closed
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented.

Comments

@surchs
Copy link
Contributor

surchs commented Nov 8, 2023

In #214 the subject_data argument return type was changed for the aggregated API mode from list to str like so:

Expected subject_data format in:

  • aggregated mode: subject_data = "protected"
  • non-aggregated mode:
{
        "sub_id": "sub-123",
        "session_id": "ses-123",
        "session_file_path": "/ds123/sub-123",
        ...
},

There are several good notes on this in the PR (e.g. here).

My concern is that changing the return type from list to string will likely surprise someone somewhere in the future. On the other hand, we also don't want to make it look as if the query just didn't return any results. So I want to open this issue in order to think about this again and maybe find a different implementation that achieves both goals

@surchs surchs added this to Neurobagel Nov 8, 2023
@surchs surchs added feat:improve flag:discuss Flag issue that needs to be discussed before it can be implemented. labels Nov 8, 2023
Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 75 days.
We have applied the _flag:stale label to indicate that this issue should be reviewed again.
When you review, please reread the spec and then apply one of these three options:

  • prioritize: apply the flag:schedule label to suggest moving this issue into the backlog now
  • close: if the issue is no longer relevant, explain why (give others a chance to reply) and then close.
  • archive: sometimes an issue has important information or ideas but we won't work on it soon. In this case
    apply the someday label to show that this won't be prioritized. The stalebot will ignore issues with this
    label in the future. Use sparingly!

@github-actions github-actions bot added the _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again label Jan 23, 2024
@surchs surchs added flag:schedule Flag issue that should go on the roadmap or backlog. and removed _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again labels Jan 30, 2024
@rmanaem rmanaem closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2024
@github-project-automation github-project-automation bot moved this to Review - Done in Neurobagel Jan 31, 2024
@rmanaem rmanaem removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented.
Projects
Archived in project
Development

No branches or pull requests

2 participants