Skip to content

Conversation

@cbowman0
Copy link
Member

Code Enhancements:

  • Add some defensive input checks in find_x_times and format_units
  • Explicitly define self.span and self.binary in class _LinearAxisTics. It was being defined in method calls.
  • Add/Improve docstrings
  • Adjust logic in applySettings to set axistLimit
  • Move minValue<maxValue check to reconcileLimits() from applySettings()

Standalone functions:

  • Tests for format_units()
  • Tests for find_x_times()
  • Tests for toSeconds()
  • Tests for sort_stacked()
  • Tests for safeArgs()
  • Tests for safeMin()
  • Tests for safeMax()
  • Tests for safeSum()
  • Tests for any()

Classes:

  • Tests for the _LinearAxisTics Class
  • Tests for the _LogAxisTics Class

cbowman0 and others added 10 commits June 29, 2016 15:52
Bug Fixes:
* Add some defensive input checks in find_x_times and format_units
* Explicitly define self.span and self.binary in class _LinearAxisTics.  It was being defined in method calls.

Standalone functions:
* Tests for format_units()
* Tests for find_x_times()
* Tests for toSeconds()
* Tests for sort_stacked()
* Tests for safeArgs()
* Tests for safeMin()
* Tests for safeMax()
* Tests for safeSum()
* Tests for any()

Classes:
* Tests for the _LinearAxisTics Class
* Tests for the _LogAxisTics Class
It was not being set to None if the axisLimit argument was positive
infinity.
Move the (minValue < maxValue) check to reconcileLimits().
Methods checkFinite() and chooseDelta() are static and defined only in
_AxisTics, so

1. An instance doesn't have to be generated to test them.

2. They can be tested once, rather than once for _LinearAxisTics and
   once for _LogAxisTics.
This method is meant to be called via applySettings(), not directly. So
call applySettings() in each test rather than reconcileLimits(). Check
its results by looking at y.minValue and y.maxValue. And add some more
tests covering cases where it really has to do some work.
As a bonus, using zip() makes the code a bit shorter.
@cbowman0
Copy link
Member Author

Graphs with these changes:
with_change

Graphs without these changes:
without_change

@codecov-io
Copy link

codecov-io commented Jun 30, 2016

Current coverage is 51.01%

Merging #1566 into master will increase coverage by 3.46%

@@             master      #1566   diff @@
==========================================
  Files            52         52          
  Lines          5796       5808    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       1120       1123     +3   
==========================================
+ Hits           2756       2963   +207   
+ Misses         2833       2635   -198   
- Partials        207        210     +3   

Powered by Codecov. Last updated by 2b02527...6e6c77a

@obfuscurity
Copy link
Member

jake

@obfuscurity obfuscurity merged commit d0295a2 into graphite-project:master Jul 1, 2016
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.

4 participants