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

Added configuration option to suppress large data warning #999

Merged
merged 3 commits into from
May 23, 2016

Conversation

robintw
Copy link
Contributor

@robintw robintw commented May 21, 2016

This is my first proper PR for this project, so please let me know if I'm doing things wrong.

Specifically, I haven't edited .ui files for Qt before, so I may have done some dumb things there...

This fixes glue-viz/glue-geospatial#6

@astrofrog
Copy link
Member

Thanks, this looks great! I'll test this out today and will merge it if everything works fine :)

One quick question to ponder - should we make the setting be the reverse of what it is here, i.e be 'emit warnings if data is large' and have it be ticked by default?

@robintw
Copy link
Contributor Author

robintw commented May 21, 2016

Personally I think it's best to have warnings on by default, and then let users turn them off, as otherwise users could easily get frustrated (or think Glue has crashed) when trying to plot large datasets. However, it's very much a matter of taste, so I'm happy either way.

@astrofrog
Copy link
Member

@robintw - just to clarify, I also think warnings should be on by default, but I meant that maybe the option should be SHOW_LARGE_DATA_WARNING and that it should be True by default?

@astrofrog
Copy link
Member

@robintw - this will need to be rebased, I think due to a small conflict in glue/config.py.

@robintw
Copy link
Contributor Author

robintw commented May 23, 2016

I must admit that I've never really understood rebasing. What should I run to do this?

@astrofrog
Copy link
Member

astrofrog commented May 23, 2016

@robintw - no problem. Rebasing means playing back your changes on top of the latest developer version. Let's say that you defined the main glue repository as being called upstream, so for instance I have:

$ git remote -v
origin  git@github.com:astrofrog/glue.git (push)
origin  git@github.com:astrofrog/glue.git (fetch)
upstream    git@github.com:glue-viz/glue.git (fetch)
upstream    git@github.com:glue-viz/glue.git (push)
...

You can start off by fetching the latest changes from upstream:

git fetch upstream

then, on your setting-ignore-large-data-warning branch, you can do:

git rebase -i upstream/master

You can review the commits in your branch in the editor that comes up:

pick 57d9694 Added configuration option SUPPRESS_LARGE_DATA_WARNING to suppress$
pick 13bc443 Added prefs GUI addition for new suppress large data warning option
...

then save to continue (without making any changes). It will then proceed to replay the changes you have on top of what's in the latest developer version.

At some point, it will complain about a conflict:

error: could not apply 57d9694... Added configuration option SUPPRESS_LARGE_DATA_WARNING to suppress displaying the warning dialog when viewing large datasets

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply 57d9694e7686b2b0b8e3c941a8dcf9edfc3f7265... Added configuration option SUPPRESS_LARGE_DATA_WARNING to suppress displaying the warning dialog when viewing large datasets

You can do git status to see the conflicted files and you will see:

    both modified:   glue/config.py

open that file, and the conflicted region will look like:

<<<<<<< HEAD
settings.add('BACKGROUND_COLOR', '#FFFFFF')
settings.add('FOREGROUND_COLOR', '#000000')
=======
settings.add('BACKGROUND_COLOR', '#FFFFFF', validator=None)
settings.add('FOREGROUND_COLOR', '#000000', validator=None)
settings.add('SUPPRESS_LARGE_DATA_WARNING', False, validator=bool)
>>>>>>> 57d9694... Added configuration option SUPPRESS_LARGE_DATA_WARNING to suppress displaying the warning dialog when viewing large datasets

Basically I had changed the developer version to remove validator=None, so just replace this block by:

settings.add('BACKGROUND_COLOR', '#FFFFFF')
settings.add('FOREGROUND_COLOR', '#000000')
settings.add('SUPPRESS_LARGE_DATA_WARNING', False, validator=bool)

then save and quit. Then you can do:

git add glue/config.py

followed by:

glue rebase --continue

and you should be all set. Once this is done, just force push your branch to GitHub. Let me know if any of the steps don't make sense!

@astrofrog
Copy link
Member

@robintw - by the way, if you run into any issues, we can also chat live at https://gitter.im/glue-viz/glue-geospatial

@robintw robintw force-pushed the setting-ignore-large-data-warning branch from 13bc443 to 31ad416 Compare May 23, 2016 13:33
…e in reverse

(ie. True is default, and shows warnings, turn off to suppress warnings)
@astrofrog astrofrog merged commit aa6f452 into glue-viz:master May 23, 2016
@astrofrog
Copy link
Member

Thanks @robintw!

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.

Warning about number of points when plotting histogram
2 participants