joystick icon indicating copy to clipboard operation
joystick copied to clipboard

Look into retaining input values between renders

Open rglover opened this issue 2 years ago • 1 comments

This is just a gotcha of HTML. On re-render, we regenerate the HTML for all elements in a component. By default, an HTML input will not retain its value because it isn't persisted in the DOM. While you can solve this by copying values over to state (a la "controlled inputs" in React), I think with a bit of finesse we can just auto-update the VDOM whenever inputs change to track the changes automatically. There could be a performance hit, but if it's throttled/debounced properly it shouldn't be an issue.

rglover avatar Feb 20 '23 20:02 rglover

@VGPReys please don't write titles like: "Solving issue #327" - instead describe what this pull request will add to the codebase 😉 and please don't make one pull request that adds more than one thing

rvhonorato avatar Aug 31 '23 08:08 rvhonorato

I don't agree with this and the code looks very messy.

json.loads cannot read keys as integers because this is the default json format.

This PR adds an anti-pattern solution as convenience and I don't think this is a good idea, what needs to be done is to re-think the output data structure and adopt a standard that is compliant with the default format.

Arguably changing 1 to "1" does achieve that but it relies heavy on python dynamic type casting and less on the output data formalism, which can be very prone to errors in the future.

up to you @mgiulini

Hi there, sorry for the late reply.

For what I see, adding the output option that generates a clustered_residues.out file like the following

Cluster 1 -> 85 87 88 161 162
Cluster 5 -> 55 57 72 74

can be useful and is in line with the other output files. As for the json, I agree that that modification is not in the purpose of the project and of this CLI in particular. Maybe @VGPReys you can load the cluster data from the clustered_residues.out file via pandas, what do you think?

mgiulini avatar Sep 04 '23 10:09 mgiulini

I don't agree with this and the code looks very messy. json.loads cannot read keys as integers because this is the default json format. This PR adds an anti-pattern solution as convenience and I don't think this is a good idea, what needs to be done is to re-think the output data structure and adopt a standard that is compliant with the default format. Arguably changing 1 to "1" does achieve that but it relies heavy on python dynamic type casting and less on the output data formalism, which can be very prone to errors in the future. up to you @mgiulini

Hi there, sorry for the late reply.

For what I see, adding the output option that generates a clustered_residues.out file like the following

Cluster 1 -> 85 87 88 161 162
Cluster 5 -> 55 57 72 74

can be useful and is in line with the other output files. As for the json, I agree that that modification is not in the purpose of the project and of this CLI in particular. Maybe @VGPReys you can load the cluster data from the clustered_residues.out file via pandas, what do you think?

Ok then let's just stick to the textual version in clustered_residues.out.

VGPReys avatar Sep 04 '23 11:09 VGPReys