Skip to content
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

ESS GUI: Warn user when data set fully masked #1444

Merged
merged 3 commits into from
Mar 19, 2020

Conversation

krzywon
Copy link
Contributor

@krzywon krzywon commented Jan 21, 2020

This PR matches 5.x up to the 4.x branch by attaching a warning to any data set that is fully masked when it is loaded. The first of the two commits has already been merged into the ESS_GUI branch, but the 2nd commit (cherry-picked here) has not.

The PR for the 4.x branch: #1356
The 1st PR for the 5.x branch: #1368

Copy link
Contributor

@pkienzle pkienzle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine. I didn't test in the GUI.

@butlerpd
Copy link
Member

Jeff to test GUI -- then approve .. .or not.

@krzywon
Copy link
Contributor Author

krzywon commented Feb 18, 2020

The 1st PR merge (#1368) that loaded in the mask was reverted along with a few other changes in a recent commit. I've restored that change here. Also, I changed DataExplorer to better handle errors linked to loaded data sets. The current behavior reads the errors and creates a string out of them, but the string isn't used anywhere.

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK

@rozyczko rozyczko merged commit b292fa7 into ESS_GUI Mar 19, 2020
@krzywon krzywon deleted the ESS_GGUI_Mask_User_Warning branch March 22, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants