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

move EPSILON to class property #19

Closed
wants to merge 1 commit into from

Conversation

perrygeo
Copy link

@perrygeo perrygeo commented Apr 6, 2016

This PR moves the EPSILON from a module-level global to a class property. Resolves #18

In order to avoid managing cache invalidation, I removed the concept of cached_properties. Most of those calculations take < 1/2 μs so I didn't see any performance reason for taking on that complexity. For example, the determinant property takes 449ns to calculate and 300ns to lookup from the cache.

Also removed the EPSILON2 global as it wasn't being used anywhere.

>>> from affine import Affine
>>> a = Affine(0.000003, 0, 1000, 0, -0.000003, 1000)
>>> a.is_degenerate
True
>>> a.EPSILON
1e-05
>>> a.determinant
-9.000000000000001e-12
>>> b = Affine(0.000003, 0, 1000, 0, -0.000003, 1000)
>>> b.is_degenerate
True
>>> a.EPSILON = a.determinant * 0.1
>>> a.is_degenerate
False
>>> b.is_degenerate  # other instances are unaffected
True

I'm not fully aware of the history of the codebase and removing the global EPSILON might be a breaking change. I'm not sold on this implementation, just a first cut.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 97.122% when pulling 616f325 on perrygeo:epsilon-local into d7395eb on sgillies:master.

@sgillies sgillies mentioned this pull request Apr 7, 2016
@perrygeo
Copy link
Author

perrygeo commented Apr 7, 2016

Closing in favor of #20

@perrygeo perrygeo closed this Apr 7, 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.

2 participants