genenetwork / genenetwork2

GeneNetwork (2nd generation)
http://gn2.genenetwork.org/
GNU Affero General Public License v3.0
34 stars 24 forks source link

Changes to sample data editing #781

Open zsloan opened 1 year ago

zsloan commented 1 year ago

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

The corresponding GN3 PR is here - https://github.com/genenetwork/genenetwork3/pull/117

zsloan commented 1 year ago

I also really don't like how those nested for loops work, but I'm not sure how else to do them. The original version of the code didn't have this problem because it only needed to care about samples with values (which would be the same in both the base and delta CSV files) and also had the CSV directly submitted instead of specific values being submitted through a form.

Basically what needs to happen is for a CSV string to be written that is essentially the base_csv but with values substituted when they were edited (what it's getting from the form). So it goes through each line and checks if edits of the value, error, or n_cases were submitted with the form.

It could maybe be made a bit more simple by converting all the edits into a single JSON value either before or after being submitted with the form, though this would also require more code and it would still need to check "do edits exist" for each sample/row. I think this would only circumvent the "if form_data.get()" checks.

On Fri, May 12, 2023, 10:46 AM jgart @.***> wrote:

@.**** commented on this pull request.

In wqflask/wqflask/metadata_edits.py https://github.com/genenetwork/genenetwork2/pull/781#discussion_r1192535789 :

@@ -705,3 +727,26 @@ def approve_data(resource_id: str, file_name: str): "warning", ) return redirect(url_for("metadata_edit.list_diffs")) + +def create_delta_csv(base_csv, form_data, sample_list):

  • base_csv_lines = base_csv.split("\n")
  • delta_csv_lines = [base_csv_lines[0]]
  • for line in base_csv_lines[1:]:

Hi, maybe there's a way to code this that doesn't have such a high cyclomatic @.***/python-code-complexity-metrics-e6c87646c1c2> complexity?

Otherwise #781 https://github.com/genenetwork/genenetwork2/pull/781 LGTM

— Reply to this email directly, view it on GitHub https://github.com/genenetwork/genenetwork2/pull/781#pullrequestreview-1424761958, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANQJGAS7VMFUMFLOQQCN4LXFZLMDANCNFSM6AAAAAAX6Q4CWE . You are receiving this because you were assigned.Message ID: @.***>