Changes to sample data editing #117

zsloan · 2023-05-11T18:50:27Z

This PR will make a couple changes to sample data editing:

Values will be editable for samples that don't currently have values. Currently you can only edit sample values for samples that already have values; this is because it just pulls the sample data from the DB and wasn't accounting for the sample list
It will be possible to submit sample data changes via an HTML table, instead of just as a CSV import

The accompanying GN2 PR is here - genenetwork/genenetwork2#781

* migrations/auth/20221110_08_23psB-add-privilege-category-and-privilege-description-columns-to-privileges-table.py: new migration * tests/unit/auth/test_migrations_add_remove_columns.py: test new migration

* .gitignore: ignore all yoyo configuration files * README.md: Update documentation * yoyo.auth.ini: stop tracking the yoyo configuration file.

* migrations/auth/20221113_01_7M0hv-enumerate-initial-privileges.py: new migration. * tests/unit/auth/test_migrations_insert_data_into_empty_table.py: test new migration.

Add table `generic_role_privileges` table to link the generic roles to the privileges they provide. * migrations/auth/20221114_01_n8gsF-create-generic-role-privileges-table.py: new migration * tests/unit/auth/test_create_table_migrations.py: test new migration * tests/unit/auth/test_migrations_indexes.py: test new migration

* Name all tests that test migrations to start with `test_migrations_`

Drop the `generic_role*` tables, since what they were envisioned for can be handled a different, (arguably) more simple way. * migrations/auth/20221114_02_DKKjn-drop-generic-role-tables.py: new migration * tests/unit/auth/test_migrations_drop_tables.py: test new migration

* migrations/auth/20221110_05_BaNtL-create-roles-table.py: modify migration * migrations/auth/20221114_03_PtWjc-create-group-roles-table.py: new migration * tests/unit/auth/test_migrations_create_tables.py: test new migration * tests/unit/auth/test_migrations_indexes.py: test new migration

* migrations/auth/20221114_04_tLUzB-initialise-basic-roles.py: new migration * tests/unit/auth/test_migrations_insert_data_into_empty_table.py: test new migration

Some roles will not be user editable to prevent inconsistencies, and possible privilege escalation. * migrations/auth/20221110_05_BaNtL-create-roles-table.py: Add `user_editable` column to table * migrations/auth/20221114_04_tLUzB-initialise-basic-roles.py: Set `group-leader` role as not user editable

* gn3/auth/authorisation/__init__.py: Add `authorised_p` decorator to be used for all function requiring authorisation. * gn3/auth/authorisation/groups.py: Add `create_group` function stub * tests/unit/auth/conftest.py: Add fixture for test users * tests/unit/auth/test_groups.py: Add tests for `create_group`

* migrations/auth/20221114_05_hQun6-create-user-roles-table.py: new migration * tests/unit/auth/test_migrations_create_tables.py: test new migration * tests/unit/auth/test_migrations_indexes.py: test new migration

* The test app and the test client are both needed as fixtures in different places

* tests/unit/auth/test_groups.py: use Flask's application context directly rather than the request context to access `g` in order to get rid of the `DeprecationWarning`.

* tests/unit/auth/conftest.py: use the functions in `gn3.auth.db` to acquire the database connection and cursor since they handle some of the basic issues like rollback and commit, meaning we do not have to explicitly handle said issues in the fixtures.

* gn3/auth/authorisation/__init__.py: delete function * gn3/auth/authorisation/checks.py: move function to `checks` module

Use specified types for privileges, roles and types rather than using strings to help with limiting bugs. * gn3/auth/authorisation/groups.py: Specify and use the `Group` type * gn3/auth/authorisation/privileges.py: Specify and use the `Privilege` type * gn3/auth/authorisation/roles.py: Specify the `Role` type. Add the `create_role` function.

* gn3/auth/authorisation/checks.py: Return results of calling the function rather than a dict of values that include the results. * gn3/auth/authorisation/groups.py: Use the newer form of `authorised_p` decorator. * tests/unit/auth/test_groups.py: Update tests

* gn3/db/rdf.py: Delete gn3.setting.SPARQL_ENDPOINT import. (sparql_query): Inject SPARQLWrapper. (get_dataset_metadata): Ditto.

* tests/unit/auth/test_roles.py: new tests.

* gn3/auth/authorisation/privileges.py: Set id to UUID type * gn3/auth/authorisation/roles.py: fix parameters to types that sqlite3 supports * gn3/auth/db.py: add logging for errors and re-raise the exception * tests/unit/auth/test_roles.py: fix test

* migrations/auth/20221116_01_nKUmX-add-privileges-to-group-leader-role.py: new migration to fix data errors. * tests/unit/auth/test_privileges.py: test privileges

Decouple the `gn3.db_utils` module from the global `flask.current_app` object, ensuring that the database uri value is passed in as a required argument to the `gn3.db_utils.database_connection` function.

* gn3/api/metadata.py: Import Template, sparql_query and RDF_PREFIXES. (get_genewiki_entries): New endpoint. * gn3/db/rdf.py: Add new constant for storing rdf prefixes. Signed-off-by: Munyoki Kilyungi <me@bonfacemunyoki.com>

Signed-off-by: Munyoki Kilyungi <me@bonfacemunyoki.com>

Consistently encode all values for the top-level keys stored in redis to avoid issues with json encode/decode

Fix bugs with setting up of the selected traits for use while filtering the search results.

During development, we need logging sometimes to help with troubleshooting problems. This commit provides a module to help set up the logging in a separate module from the app module.

…herwise after the recent changes to the DB

There's probably a better way to fix this query (it was previously returning each sample twice), but DISTINCT was the easiest way I could come up with

This is necessary in order to allow for editing the values of samples that don't currently have values

BonfaceKilz

@zsloan thanks for pinging me to review this. I've left some minor comments that you could look at. Also, generally, you could use tests to try and poke holes at weird edge cases.

BonfaceKilz · 2023-05-12T09:03:27Z

gn3/db/datasets.py

+    the "official" sample list is stored
+    """
+
+    genofile_path = os.environ.get("GENENETWORK_FILES") + "/genotype/" + group + ".geno"


nitpick: genofile_path is a constant, and as such, upper-case it.

BonfaceKilz · 2023-05-12T09:04:34Z

gn3/db/datasets.py

+        genofile = open(genofile_path)
+
+        for line in genofile:
+            line = line.strip()
+            if not line:
+                continue
+            if line.startswith(("#", "@")):
+                continue
+            break
+
+        headers = line.split("\t")
+
+        if headers[3] == "Mb":
+            samplelist = headers[4:]
+        else:
+            samplelist = headers[3:]
+        return samplelist


issue: When opening a file, use a context-manager, in this case, with open .... This way, you don't have to worry about closing any hanging file connections should something unforeseen happens.

BonfaceKilz · 2023-05-12T09:06:02Z

gn3/db/datasets.py

+        "SELECT Name "
+        "FROM InbredSet "
+        "WHERE "
+        "InbredSet.Id = %(group_id)s")


question: is this escaped? I thought just using simple "%s" like so:

"SELECT * FROM .... WHERE ... = %s", value

should escape things.

BonfaceKilz · 2023-05-12T09:07:23Z

gn3/db/datasets.py

+        if res:
+            return res[0]
+        return None


nitpick: @arunisaac write some monadic constructs to deal with None values when doing SQL queries. You could look at those. If you get stuck, we could chat over on Matrix and I could show you how to use them.

BonfaceKilz

@zsloan thanks for pinging me to review this. I've left some minor comments that you could look at. Also, generally, you could use tests to try and poke holes at weird edge cases.

fredmanglis and others added 30 commits November 10, 2022 16:14

Tests: Generalise tests for adding/removing columns to/from tables

5ee7eee

Migrations: Add columns to 'privileges' table

3bc52c3

* migrations/auth/20221110_08_23psB-add-privilege-category-and-privilege-description-columns-to-privileges-table.py: new migration * tests/unit/auth/test_migrations_add_remove_columns.py: test new migration

Documentation: Explain how to run migrations

0bdc5bc

* .gitignore: ignore all yoyo configuration files * README.md: Update documentation * yoyo.auth.ini: stop tracking the yoyo configuration file.

Migrations: Migration for initial privileges

2a29b82

* migrations/auth/20221113_01_7M0hv-enumerate-initial-privileges.py: new migration. * tests/unit/auth/test_migrations_insert_data_into_empty_table.py: test new migration.

Rename files for consistency

f1f04c5

* Name all tests that test migrations to start with `test_migrations_`

Migrations: Create the group-leader role and give it privileges

f3295a7

* migrations/auth/20221114_04_tLUzB-initialise-basic-roles.py: new migration * tests/unit/auth/test_migrations_insert_data_into_empty_table.py: test new migration

Migrations: Create user_roles table

7ef618e

* migrations/auth/20221114_05_hQun6-create-user-roles-table.py: new migration * tests/unit/auth/test_migrations_create_tables.py: test new migration * tests/unit/auth/test_migrations_indexes.py: test new migration

tests: split test app and test client

baf7980

* The test app and the test client are both needed as fixtures in different places

auth: Implement create_group

6b964b9

tests: Fix cause of DeprecationWarning

f640f3f

* tests/unit/auth/test_groups.py: use Flask's application context directly rather than the request context to access `g` in order to get rid of the `DeprecationWarning`.

tests: typo: fix migration script file name

9aee64c

pylint: Fix linting errors.

b0d4aeb

auth: Reorganise package - move function to checks module

b1ee095

* gn3/auth/authorisation/__init__.py: delete function * gn3/auth/authorisation/checks.py: move function to `checks` module

auth: Add some typing information to the functions

3dc9a8a

pylint: Fix a myriad of linting errors

d043cd5

Add missing module __init__.py to mitigate the mypy errors

a0e654f

auth: Add create_group_role function.

bec9a1d

Inject SPARQLWrapper as a parameter

749537c

* gn3/db/rdf.py: Delete gn3.setting.SPARQL_ENDPOINT import. (sparql_query): Inject SPARQLWrapper. (get_dataset_metadata): Ditto.

tests: Test role creation

c18e248

* tests/unit/auth/test_roles.py: new tests.

auth: fix bugs in the code

25da023

* gn3/auth/authorisation/privileges.py: Set id to UUID type * gn3/auth/authorisation/roles.py: fix parameters to types that sqlite3 supports * gn3/auth/db.py: add logging for errors and re-raise the exception * tests/unit/auth/test_roles.py: fix test

tests: Add tests for privileges

366ad03

* migrations/auth/20221116_01_nKUmX-add-privileges-to-group-leader-role.py: new migration to fix data errors. * tests/unit/auth/test_privileges.py: test privileges

pylint: fix some linting errors

58dcae9

fredmanglis and others added 24 commits April 15, 2023 19:35

Decouple gn3.db_utils from flask.current_app.

dbf0f9f

Decouple the `gn3.db_utils` module from the global `flask.current_app` object, ensuring that the database uri value is passed in as a required argument to the `gn3.db_utils.database_connection` function.

auth: Delete code that is no longer used

85abf12

Hook up code to use external search script for phenotypes

8af8105

Create new endpoint for fetching GeneRIF entries

d047792

* gn3/api/metadata.py: Import Template, sparql_query and RDF_PREFIXES. (get_genewiki_entries): New endpoint. * gn3/db/rdf.py: Add new constant for storing rdf prefixes. Signed-off-by: Munyoki Kilyungi <me@bonfacemunyoki.com>

Update the default SPARQL endpoint

b06a199

Signed-off-by: Munyoki Kilyungi <me@bonfacemunyoki.com>

Fix typo, os.environment -> os.environ

03bbc35

Signed-off-by: Munyoki Kilyungi <me@bonfacemunyoki.com>

auth: Consistently JSON encode values.

0958648

Consistently encode all values for the top-level keys stored in redis to avoid issues with json encode/decode

auth: Pass --per-page and --selected options.

280f797

auth: phenotypes - provide default empty list for selected traits.

85610fc

auth: Setup selected traits correctly

9a0c6fd

Fix bugs with setting up of the selected traits for use while filtering the search results.

Fix minor linting and typing issues

2a7e8b7

oauth2: Link the phenotype traits to user groups.

735adc0

auth: Fetch user group phenotypes not attached to a resource.

1fb5633

Setup module for logging

95f067a

During development, we need logging sometimes to help with troubleshooting problems. This commit provides a module to help set up the logging in a separate module from the app module.

auth: Attach linked data to specific resources.

2f6f54e

logging: Set LOG LEVEL on root logger to enable logs

993420e

Add function for retrieving samplelist from .geno file

c99d688

Add function for retrieving phenotype sample data as dict

6bf4bfd

Add function for retrieving group name (given group ID)

e55bcc5

Commit update/insert queries, since they don't seem to be executed ot…

c624900

…herwise after the recent changes to the DB

Fix retrieve_sample_list function to correctly get genofile_path

b39bf7e

Fix get_trait_sample_data function

2505677

Fix sample data CSV query to only fetch distinct samples

929545b

There's probably a better way to fix this query (it was previously returning each sample twice), but DISTINCT was the easiest way I could come up with

Include all samples in trait sample data

1b75e8e

This is necessary in order to allow for editing the values of samples that don't currently have values

zsloan requested a review from BonfaceKilz May 11, 2023 18:50

zsloan self-assigned this May 11, 2023

zsloan mentioned this pull request May 11, 2023

Changes to sample data editing genenetwork/genenetwork2#781

Open

BonfaceKilz approved these changes May 12, 2023

View reviewed changes

pjotrp force-pushed the main branch from c08508c to fbc8d03 Compare August 6, 2023 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to sample data editing #117

Changes to sample data editing #117

zsloan commented May 11, 2023

BonfaceKilz left a comment

BonfaceKilz May 12, 2023

BonfaceKilz May 12, 2023

BonfaceKilz May 12, 2023

BonfaceKilz May 12, 2023

BonfaceKilz left a comment

Changes to sample data editing #117

Are you sure you want to change the base?

Changes to sample data editing #117

Conversation

zsloan commented May 11, 2023

BonfaceKilz left a comment

Choose a reason for hiding this comment

BonfaceKilz May 12, 2023

Choose a reason for hiding this comment

BonfaceKilz May 12, 2023

Choose a reason for hiding this comment

BonfaceKilz May 12, 2023

Choose a reason for hiding this comment

BonfaceKilz May 12, 2023

Choose a reason for hiding this comment

BonfaceKilz left a comment

Choose a reason for hiding this comment