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

Symbol Gallery #304

Merged
merged 3 commits into from Feb 13, 2017
Merged

Symbol Gallery #304

merged 3 commits into from Feb 13, 2017

Conversation

jrleeman
Copy link
Contributor

@jrleeman jrleeman commented Feb 3, 2017

Adding a symbol gallery per #166 - will make this into part of the docs instead of an example unless there's objection. Notebook for prototyping, but I'm pretty happy with the look of the output.

Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Just a few things:

  1. Rebase and reword all commit messages to remove 'WIP'
  2. Squish together the commits about the fontsize parameter
  3. No notebooks--remember, all examples are .py files now

The images themselves look great!

@dopplershift
Copy link
Member

dopplershift commented Feb 3, 2017

Once you commit the .py version of the example, rebase and squash/whatnot to eradicate any history of the notebook so we don't commit those images to the repo.

Do you want to take a crack at including the images in the docs for plotting themselves, or do you want to wait on that?

@jrleeman
Copy link
Contributor Author

jrleeman commented Feb 3, 2017

I'll have a crack at it - I just left it in the notebook while playing and will kill all of that once I get the integration with mpl working.

@dopplershift
Copy link
Member

Ah, ok. Well in that case, sail on. (And if you just wan't feedback and not an in-depth review, put [WIP] in the title of the PR)

@jrleeman
Copy link
Contributor Author

jrleeman commented Feb 3, 2017

Got it. I'll plug away and see where it gets. Hopefully can have it in the docs shortly.

@jrleeman
Copy link
Contributor Author

jrleeman commented Feb 4, 2017

Oddly enough, this works, but the figure titles do not render. Something to look into.

@jrleeman
Copy link
Contributor Author

jrleeman commented Feb 6, 2017

Any thoughts on how this makes the docs look? I'll clean up the history once we're tuned in.

@jrleeman jrleeman force-pushed the SymbolGallery branch 2 times, most recently from c95cc57 to a7ccdf9 Compare February 6, 2017 21:19
@jrleeman
Copy link
Contributor Author

jrleeman commented Feb 8, 2017

After merging #310 and a rebase, this should be passing tests and ready for final review.

Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Can you squash the commits to remove the image change?

Also, can you add a couple tests for the added features? The len(symbol_mapper) code is completely uncovered, and it would be nice to have a test for the fontsize stuff as well.

dopplershift and others added 2 commits February 10, 2017 14:46
Show all possible symbols by plotting in the docstring of plot_symbol.
@dopplershift
Copy link
Member

So make the default threshold 0.105. It's a bit higher than I want, but I just don't feel like making a special threshold on windows. Should be plenty tight to catch problems, which is the real purpose.

@dopplershift dopplershift merged commit 414dea5 into Unidata:master Feb 13, 2017
@dopplershift dopplershift modified the milestone: Spring 2017 Feb 21, 2017
@jrleeman jrleeman deleted the SymbolGallery branch February 27, 2017 21:45
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.

None yet

2 participants