biolab / orange3-educational

🍊 🎓 Educational widgets for machine learning and data mining in Orange 3.
Other
27 stars 20 forks source link

Update owrandomdata.py #164

Closed jcmhk closed 10 months ago

jcmhk commented 11 months ago

I forgot to change ParameterDef("High bound", "scale", 1, any_float) to ParameterDef("Range", "scale", 1, any_float) in my last pull request

Issue
Description of changes
Includes
jcmhk commented 11 months ago

I forgot to change ParameterDef("High bound", "scale", 1, any_float) to ParameterDef("Range", "scale", 1, any_float) in my last pull request

janezd commented 11 months ago

Thank you for this! Apparently nobody ever noticed that the interval is wrong.

I would suggest a change, though. For me, it seems more intuitive (from the user perspective) to specify the interval boundaries rather than the lower bound and the range. So I would keep the interface, but fix the code behind.

On the first glance, it should suffice to just replace rvs = stats.uniform.rvs with

    @staticmethod
    def rvs(loc, scale, size):
        return stats.uniform.rvs(loc, scale - loc, size=size)

and leave everything else as it was before. But please check that this is indeed the case -- I haven't tried it.

We should, in principle, also rename loc and scale to, say, lower and upper, but then any saved workflows with old names would stop working unless we add the migration code ... and I don't think its worth the effort, so we could just keep the existing (though wrong names).

So, I would just ask you to try the above code snippet instead of your fix -- and, of course, change it if necessary.

Once again thanks for spotting this and suggesting a fix.

jcmhk commented 11 months ago

Hello, I am going to try

jcmhk commented 11 months ago

I just tested your solution, it solves the problem. Thank you so much