search

vMeijin / pySmithPlot

Matplotlib extension for creating Smith charts with Python
127 stars 73 forks source link

Hello, and thanks for writing this awesome library! #7

Closed rpcope1 closed 9 years ago

rpcope1 commented 9 years ago

I looked through library and tried to clean things that were potentially dangerous (like {} as a default argument, which may not do what you think it should do), and added some TODOs where things might be cleaned. I also added a setup.py using setuptools to make this easy to install and use. Will study this library further as I make use of it more.

I'm not sure if this is something you're actively maintaining, but I would like to help if there's anything you wanted to add or clean up on this library. I am going to be using it for my microwave instrumentation class, and if I can help out at all on this (or if you are looking for a new maintainer) I would be glad to help.

I hope all is well.

Robert Cope

mradway commented 9 years ago

Hello Robert,

I'm not affiliated with the project, and I can't speak to your changes, but it does appear that this library could benefit from a more active maintainer. I'd encourage you to go nuts on the code, periodically submitting pull requests against this repo.

Come to think about it, my little side project also has an outstanding pull request on it. Hopefully I can get to it in the next few days.

Matt

On Thu, Oct 9, 2014 at 10:00 PM, Robert P. Cope notifications@github.com wrote:

I looked through library and tried to clean things that were potentially dangerous (like {} as a default argument, which may not do what you think it should do), and added some TODOs where things might be cleaned. I also added a setup.py using setuptools to make this easy to install and use. Will study this library further as I make use of it more.

I'm not sure if this is something you're actively maintaining, but I would like to help if there's anything you wanted to add or clean up on this library. I am going to be using it for my microwave instrumentation class, and if I can help out at all on this (or if you are looking for a new maintainer) I would be glad to help.

I hope all is well.

Robert Cope

You can merge this Pull Request by running

git pull https://github.com/rpcope1/pySmithPlot master

Or view, comment on, or merge it at:

https://github.com/vMeijin/pySmithPlot/pull/7 Commit Summary

  • Looked through library and tried to clean things that were potentially dangerous (like {} as a default argument, which may not do what you think it should do), and added some TODOs where things might be cleaned. I also added a setup.py using setuptools to make this easy to install and use. Will study this library further as I make use of it more.

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/vMeijin/pySmithPlot/pull/7.

rpcope1 commented 9 years ago

Matt, that's probably a good call; I probably should have looked, and I would have seen your pull request. I'll see what I can do about digging into the code here and submitting some patches, as it seems like it might be a little rough around the edges.

vMeijin commented 9 years ago

Hi, I know I have not maintained this project for a long time now because I did not need it by myself. If anyone wants to contribute I would appreciate it. I will take a look at the pull request and merge it.

rpcope1 commented 9 years ago

Cool! Thanks dude! :) :+1:

vMeijin commented 9 years ago

By the way, if you wonder about the code complexity here are some points why it was necessary:

After all this optimization the exported vector smithcharts (eg as pdf) are now not much bigger and slower than a normal 2D plot and have the same look-and-feel. But there are still some bugs:

mradway commented 9 years ago

Thanks, that clarifies some things with regard to your design decisions. Thanks for the project!

On Fri, Oct 10, 2014 at 4:53 PM, Paul Staerke notifications@github.com wrote:

By the way, if you wonder about the code complexity here are some points why it was necessary:

  • Smithcharts project the complete real-positive complex plane on a circle -> simple linear interpolation has a really bad resolution for larger numbers. Solution: non-linear interpolation
  • Straight lines become cirles. Simply creating a Circle out of many small straight lines for the grid results in a large amount of points and segments to be drawn and saved -> very large and slow vector graphics. Solution: optimize grid and usage of 3-point arcs
  • the library should fit in the matplotlib framework smoothly which generates some overhead
  • matplotlib hase some issues with draw order and positioning

After all this optimization the exported vector smithcharts (eg as pdf) are now not much bigger and slower than a normal 2D plot and have the same look-and-feel. But there are still some bugs:

  • Some major and minor gridlines overlap (visible if the majorgrid is transparent)
  • issues with interpolation (interpolation of complex values is not clear and defined)
  • bad code (I am not a good programmer)
  • integration into the matplotlib framework (it works for me but not necessarily for you)
  • 'hacks' (like function injection) to realize some features
  • python 3 compatibility not tested
  • check for invalid input data is not completely implemented

— Reply to this email directly or view it on GitHub https://github.com/vMeijin/pySmithPlot/pull/7#issuecomment-58729848.