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

Enable grid and cursor in plots #52

Open
wants to merge 3 commits into
base: InNextRelease
Choose a base branch
from

Conversation

eldarkg
Copy link
Contributor

@eldarkg eldarkg commented Jun 7, 2020

see

@eldarkg eldarkg changed the title Enable grid in plots Enable grid and cursor in plots Jun 7, 2020
@PetePupalaikis
Copy link
Contributor

Hi Eldar:

Thanks for contributing! This is a feature that I'd wanted to add (grids) and the panning with cursor readout was always bad before. But I'll probably want to add the showing of grids to the preferences, which you might not know how to do (easily).
I'd prefer, if possible, that you gave me a pull-request for a feature branch off of InNextRelease. Then, I'd simply pull it down, make the preferences changes, and then merge with InNextRelease. Otherwise, if you don't want to do that, give me a few days to evaluate.
When you sent this, I was just in the middle of trying to update all of the documentation for an imminent new release, since I've added a lot of stuff. If possible, we can incorporate.
Pete

@PetePupalaikis
Copy link
Contributor

This looks great. My comments:

  • I don't like that panning is disabled when the plot window comes up (I purposely enable that because that is always the first thing I'd want to). So I'd want panning to be on by default, and then to do something special to annotate the plots.
  • We'll need to add something to the toolbar to switch from panning to annotating.
  • And like I said previously, will want to add whether the grids are showing to the preferences.
  • We'll have to add same functionality in s-parameter viewer window.
  • Is there a way to augment the matplotlib toolbar with this feature (currently, if I switch to panning and then want to annotate, I can't see how to do that).

@PetePupalaikis
Copy link
Contributor

One final question: Have you verified that this also works in Python 2.7? I'm still maintaining that this works in that version of Python for the near future.

@eldarkg
Copy link
Contributor Author

eldarkg commented Jun 8, 2020

Sorry I have small time in this week. You can do it ourself.
I don't test it on python 2.7

@PetePupalaikis
Copy link
Contributor

I took the first part of your commit to add grids and added this to the preferences and the GUI. Grids are off by default, but when a user turns them on, they are turned on in the preferences for all future plots. I also updated the help system file (but did not rebuild the help system yet).
I found the remainder of your commits (the two to enable the plot cursors) had some problems:

  1. (the main problem) - having them on by default means I can't pan or zoom, and panning or zooming turns it off forever. Need some way to solve that (I think by adding a toolbar element to the NavigationToolbar2Tk class

  2. There are some bugs - namely usage of the plot cursors before assignment when in variable line width mode.

  3. While the descriptor box can be dragged, I'd really want to drag the arrow head also -- that doesn't seem possible.

Anyway, since I recently released the code, I rebased your changes for work on at some point.

@eldarkg
Copy link
Contributor Author

eldarkg commented Jun 15, 2020

Ok

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.

2 participants