zalando / expan

Open-source Python library for statistical analysis of randomised control trials (A/B tests)
MIT License
335 stars 50 forks source link

Think of a more elegant way to handle SGA #162

Open gbordyugov opened 7 years ago

gbordyugov commented 7 years ago

The current way of doing it revolves around passing an external data frame to the (apparently private) _delta() method, which goes a bit against the idea of encapsulation

https://github.com/zalando/expan/blob/dev/expan/core/experiment.py#L102

daryadedik commented 7 years ago

Can you explain a bit more about what is the problem with passing data to _delta() from sga?

gbordyugov commented 7 years ago

It's about encapsulation. _delta() is a (private) method of the Experiment class, it's supposed to perform operations on its fields instead of taking arbitrary (and possibly not verified) data frames from the outside world.

Which one do you like better:

class Complex(object):
  def __init__(self, x, y):
    self.x, self.y = x, y

  def abs(self):
    return sqrt(self.x*self.x + self.y*self.y)

or

class Complex(object):
  def __init__(self, x, y):
    self.x, self.y = x, y

  def abs(self, x, y):
    return sqrt(x*x + y*y)

?

daryadedik commented 7 years ago

thanks for clarifying! It makes sense, just didn't get it from the first sight.