-
Notifications
You must be signed in to change notification settings - Fork 439
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
[Feature request] Allow Scientist::Experiment
classes to *not* be default Scientist experiment class
#162
Comments
So I went ahead and tried to take a stab at it - but I realized that there is something eerily similar here! (code link) That being said - I can do something like
Though that kind of exposes class internals (e.g. if you ever want to switch clients off from using |
Linked PR - #163 |
Hi @zerowidth I noticed that you recently committed to the repo. I'm wondering if you could take a look at the issue and give the PR a review? If it doesn't make sense, or if it's not valid, I'm happy to close it. But it's been sitting for 3 months open and I'd like to try and address anything / bring it to a close if possible. |
I think the "creating class make it default" is a weird pattern. It should be clearer. Like a Scientist initializer that specificy the custom default experiment. Related : #92 |
Hey @benoittgt I'd love to merge the PR I had set up or keep pushing through it. Last I tagged someone, I wasn't sure who / didn't get any comments so I wasn't sure what to do. |
Hey guys, any update here? IMO the most flexible approach would be being able to pass an # pass the class in `opts`:
Scientist.science 'my-experiment', experiment_class: MyExperiment do |e|
end # perhaps we could just pass an instance of Experiment
# OR the experiment name to use the default Experiment implementation?
# Use the passed instance
Scientist.science MyExperiment.new('my-experiment') do |e|
end
# Use the default implementation
Scientist.science 'my-experiment' do |e|
end |
@madejejej Found this issue because we're also facing some friction with the flexibility of the One thing I wanted to call out in your example was that if you're instantiating the experiment yourself like A feature that could be cool is being able to implement the class MyExperiment
include Scientist::Experiment
def initialize(opts = {})
super("my-experiment")
@foo = opts[:foo]
end
def enabled?
end
def control
end
# I think this is an array in the library implementation, but just an example
def candidate
end
end and then elsewhere you can just run MyExperiment.new(foo: "bar").run |
Hey folks, I'm also unsure how best to structure things so that multiple experiments can be run at a time, and I find the "make class default when Scientist::Experiment included" a strange pattern I agree with @madejejej's suggestion to pass an I'd also suggest that classes that include
It could also be that I'm not understanding the purpose and usage of |
@alexpech12 So I ended up taking a stab at an alternate implementation of a Ruby experimentation lib that's heavily inspired by Scientist, but addresses the concerns you and others in this thread have raised: https://github.com/omkarmoghe/lab_coat. I think it "plays nice" with Rails applications, which is my use-case as well. Just mentioning it here in case you or anyone else in this thread finds it useful. Open to feedback as well. |
Thanks @omkarmoghe, I'll check it out! I'm also exploring an alternative where I avoid using |
Hey y'all - I mentioned earlier in my comment that I'd love to advance or work on this in any way possible, but it doesn't seem like there's much traction on this. I'm not sure how to get maintainer eyes on it (and tagged one earlier as well), but am always happy to keep pushing my current iteration / branch forward. The PR I linked #163 was my first stab - but definitely not meant to be the final solution. |
cc @zerowidth @ekroon thoughts? |
@danielvdao If you do not want to override the default experiment class and you want to just run an experiment without calling Scientist.run "experiment-name" do |e|
# do the usual things normally run in a "science" call.
end |
@ekroon this still uses an implicit, single, default experiment class. This doesn't allow flexibility to, for example, run experiments that have different publish, raise, and enabled behaviors. I suggest @omkarmoghe's suggestion is the cleanest path forward. Keep the |
@ozydingo If you want to publish different for a specific class you could do something like is done here:
class MyExperiment
include Scientist::Experiment
Scientist::Experiment.set_default(nil)
# Implement required methods
end
@ex = MyExperiment.new
@ex.try { ... }
@ex.run |
I'm wondering what the original inspiration for making any class that
include
s the module the default experiment, code here. Is it possible to create a class thatinclude
s theScientist::Experiment
module and not make it the default scientist experiment?Looking here it doesn't seem like it is possible.
From docs as well:
The reason I ask is because recently we ran into an issue with our Rails app:
Scientist
viainclude
Scientist
with the default experiment, but was not publishing anything, oops.include
dScientist::Experiment
, but since our tests don't eager load our classes, it was a flaky test and not surfaced during our build.So a few questions I was wondering:
include Scientist
and override the default methods for that class? If so, the only reason I held back from doing so is better separation of responsibilities. It was going to look a bit messy to clog up one class's specs with experiment specs and I'd rather separate the two easily 🤔I looked into it a bit myself, but it doesn't seem like there is a reasonably clean way. Although I think we can add a class method (something like):
What do y'all think? If so - I can try to take a stab at it.
The text was updated successfully, but these errors were encountered: