search

pythonprofilers / memory_profiler

Monitor Memory usage of Python code
http://pypi.python.org/pypi/memory_profiler
Other
4.38k stars 379 forks source link

Generally low code quality #221

Open hnykda opened 5 years ago

hnykda commented 5 years ago

The whole package is written in a quite disturbing way. Various problems:

  1. argparse is not using subparsers (with set_default(func...) and rather some "ad-hoc" home-made subparsers
  2. tests are not using unittest framework (so makefile would be almost obsolete if we used unittest with cli discover option)
  3. prints are used instead of logging (and sometimes mixed together in one module)
  4. CLI is not decoupled from the business logic, it's e.g. very hard to test the functionality of mprof.py without overriding sys.argv. There is almost no separation of concerns, everything living in one big module in huge functions which are hard to test and reuse.
  5. code style violates pep8 (long lines, indentation, triple quotes for regular strings...)
  6. functionality could be split into smaller modules doing one thing.

All of these make contribution harder and less interesting on otherwise very handy package.

astrojuanlu commented 5 years ago

Hi @hnykda, thank you for caring about the code quality of memory-profiler. Some of the issues you highlighted are easy to fix, so I encourage you to submit pull requests for those. I think it's a much better use of your energy and maintainers time.

hnykda commented 5 years ago

Yeah, I already tried to do so with #220 where I am introducing unittest for example. That's the maximum my current time allows (for now). Nevertheless, I wanted to report it here hoping that others could get inspired and maybe incorporate the above into their future code.