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

Justin extensions #34

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

justin808
Copy link
Contributor

I'd like to know if any of these changes are helpful (or even incorrect). I'm using these changes in a production system.

Review on Reviewable

Second comparison of this method in awesome_print/formatter.rb causes
the crash.

Without this fix, pry + awesome_print cannot be used, as even a simple
case such as

[1, 2, 3].to_scale

will crash

    # Catch all method to format an arbitrary object.
    #------------------------------------------------------------------------------
    def awesome_self(object, type)
      if @options[:raw] && object.instance_variables.any?
        awesome_object(object)
      elsif object == ENV
        awesome_hash(object.to_hash)
      else
        colorize(object.inspect.to_s, type)
      end
    end
@clbustos
Copy link
Owner

Let me check all the commits. Are many!

I bumped the version to 1.4.1.

* master:
  Updated README
  Fixed the implementation error of throw an error on comparing vector with another object, when only false should be sended
  Checking awesome_print error
  Updated license to BSD-3 (I could change it later). Added stable documentation
  Updated references
  Changed Manifest, dependences and README.rd
  Deleted bin. Added version file
  Updating to 1.4.0
  Raising in equality precludes working with AwesomePrint

Conflicts:
	.gitignore
	Gemfile
	Gemfile.lock
	lib/statsample/version.rb
@justin808
Copy link
Contributor Author

@clbustos I just submitted the merge and I'm testing on my own production product.

@justin808
Copy link
Contributor Author

Just tested on my own build. Seems fine. Not sure about Travis.

This is the computation that fails
def t_r(r,size)
  r * Math::sqrt(((size)-2).to_f / (1 - r**2))
end
@justin808
Copy link
Contributor Author

@clbustos Any status of this? I had to add another commit to indicate that an exception is being caused by an r value greater than 1. Any idea how this can happen?

@justin808
Copy link
Contributor Author

@clbustos I figured out the issue. We had two series with the same values. Statsample shouldn't crash in that case.

@justin808
Copy link
Contributor Author

@clbustos Any word on merging my changes?

@agarie
Copy link

agarie commented Mar 18, 2015

Hi @justin808, thanks for the pull request! We're currently in the process of centralizing SciRuby's gems in the organization repositories. Can you reopen your PR on sciruby/statsample?

Thanks! I'll take a look at your PR as soon as I finish moving the other gems' issues there. :)

@justin808
Copy link
Contributor Author

How do I re-open the PR? Maybe open a new PR?

@agarie
Copy link

agarie commented Mar 18, 2015

Yes, I think that's the easiest way. I couldn't find anything in GitHub's documentation. :(

@justin808
Copy link
Contributor Author

Here's the PR....It is OPEN. I get to this screen when i tried to recreate it.

@agarie
Copy link

agarie commented Mar 18, 2015

Maybe if you close this pull request first? Then, in the "create new PR" interface, you can select to which repository you want to send the PR -- just select SciRuby/statsample.

I'm sorry, I don't know a better way to do this. =(

@justin808
Copy link
Contributor Author

SciRuby#8

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.

3 participants