Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove +/-1 and 3 sigma lines from P(r) plots #2447

Closed
smk78 opened this issue Feb 28, 2023 · 1 comment · Fixed by #2448
Closed

Remove +/-1 and 3 sigma lines from P(r) plots #2447

smk78 opened this issue Feb 28, 2023 · 1 comment · Fixed by #2448
Assignees
Labels
Defect Bug or undesirable behaviour

Comments

@smk78
Copy link
Contributor

smk78 commented Feb 28, 2023

In merging #2443 it was noted that the functionality review noticed the new sigma bounds also appeared on P(r) plots. They should not. This needs to be addressed.

@lucas-wilkins says that the Corfunc plots should be unaffected.

@smk78 smk78 added the Defect Bug or undesirable behaviour label Feb 28, 2023
@krzywon
Copy link
Contributor

krzywon commented Feb 28, 2023

The lines are also applied to the polydispersity plots. The underlying issue is the plot role and how it is applied. There are currently four 'roles' defined in the plotting routines. 'Data' is for imported data, 'Default' is for fits to data, 'Deleteable' is for orphaned data, and 'Residual' is for residual plots but is used in a few other places.

Currently, when any plot is generated, what is being plotted must either be a data set or linked to an underlying data set or theory. Under most circumstances, the data and secondary plot or displayed together. Residual plots are standalone so they have their own role, but the residual role is the only role that currently exists to plot something on its own. The P(r) and polydispersity plots are tagged as 'Residuals' for this purpose, however, this is a misnomer. This only became a problem now that we are trying to add functionality based on the plot role.

Short-term we need to add one or more new plot roles for items that should not be shown with other data (and create an Enum to house these roles). There are also a series of conditionals that use plot roles that will need to be updated in DataExplorer.py. I've already started this, but I also think @butlerpd was looking into it as well, so I'm assigning the two of us to this ticket.

Long-term we need to rethink how plotting is performed to remove the link between the data explorer and plotting which might help remove the link between the imported data set and the theory plots.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Defect Bug or undesirable behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants