-
Notifications
You must be signed in to change notification settings - Fork 1
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
Metamist release #554
Metamist release #554
Conversation
Add missing npm run compile step in documentation and fix the spellin…
* Improve set-up documentation + aux changes Signed-off-by: Michael Franklin <illusional@users.noreply.github.com> * Fix default ped path for generate_data * Apply review feedback * Sneaky sneaky --------- Signed-off-by: Michael Franklin <illusional@users.noreply.github.com> Co-authored-by: Michael Franklin <illusional@users.noreply.github.com>
* Add set default role in instructions Missed step from me, not sure why it worked initially, but it definitely didn't work without it * Add more semicolons
* Address mypy errors * More linting updates * Two tests to fix checks * Rejig lint to build package to mypy scripts * Add strawberry[debug-server] requirement * Revert to the OpenApiGenNoneType for tests * Add extra import --------- Co-authored-by: Michael Franklin <illusional@users.noreply.github.com>
* Add participant phenotypes to graphql * Add test for graphql phenotypes * Fix unrelated linting issues * Slight linting updates * PR cleanup --------- Co-authored-by: Michael Franklin <illusional@users.noreply.github.com>
Prevent "The following action(s) uses node12 which is deprecated" warnings by updating to the current releases of the Actions used. (Node.js 12 is scheduled for removal from Actions runners next month.) Use the current releases instead of `@main` too, as we don't want to be affected by bleeding-edge bugs. Rewrite ::set-output as a write to $GITHUB_OUTPUT instead; the ::save-state and ::set-output commands are also deprecated.
@@ -189,6 +189,7 @@ def _project_summary_process_sample_rows( | |||
created_date=str(sample_id_start_times.get(s['id'], '')), | |||
sequencing_groups=sg_models_by_sample_id.get(s['id'], []), | |||
non_sequencing_assays=filtered_assay_models_by_sid.get(s['id'], []), | |||
active=bool(ord(s['active'])), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is happening here? Isn't s['active']
already a bool variable?
Not sure I understand why using ord()
and bool()
will work on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly not, encode/databases#347
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! 🤔 So in the schema the active
field is a BOOLEAN type, which in MySQL is a synonym for TINYINT(1), which when returned comes out like BIT... so we need this conversion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me ✅
Requires #553 before merging