rubyworks/facets

Proposal: Enumerable#squeeze, String#rotate etc #80

cielavenir posted onGitHub

https://github.com/cielavenir/is07kuno/blob/master/libciel.rb

I'm wondering if I'm going to publish a gem, but I'm glad if merged into facets.


posted by cielavenir almost 11 years ago

I will look them over and get back to you. Thanks.

posted by trans almost 11 years ago

Hi @cielavenir,

Thanks for your submissions! I looked them over and have some comments. In order to make it easier to incorporate your contributions into Facets, it would help if you could make the coding style and structure consistent with the rest of Facets. You should fork Facets and add your methods on a branch. Some guidelines for your methods (see CONTRIBUTE.md for more):

Here are my comments on your methods functionality:

  • Object#extract looks good!
  • Kernel#zip I'm not sure what this does-- I also don't think it should be on Kernel, since this is mixed in everywhere. Can you provide an example use-case?
  • Enumerable#squeeze looks good!
  • Enumerator::Lazy#squeze looks good!
  • String#rotate/rotate! looks good!
  • Hash#exists_rec? I like this concept, but I think it needs to be renamed (fetch_nested?). It returns the actual value rather than a Boolean, so a ? is not a good last entry. I also think the arglist should be splatted so that it's not necessary to pass an array. Finally, I think it should accept a block like Hash#fetch that provides an alternative in the case that the record doesn't exist.
def fetch_nested *keys
  begin
    keys.reduce(self) { |accum, k| accum.is_a?(Hash) && accum.fetch(k) }
  rescue KeyError, NoMethodError
    block_given ? yield(*keys) : nil
  end
end
  • Hash#patch I can't tell, does this method do anything Hash#merge doesn't do?
  • Array#permutation2 can you be more specific about what this does? I tried calling both permutation(4) and permutation2(4) on [1,2,3,4,5] and the results were identical.

In general you've got good stuff here and I'd like to see it included!

posted by smackesey almost 11 years ago

Thank you for comments. You can see testcases here: https://github.com/cielavenir/ruby_libciel/blob/master/spec/libciel_spec.rb (unfortunately this is written using rspec, so it seems I need to convert it to lemons)

  • DBI Actually this is something I used in my database experiment. Anyway it might not have good quality to be included to facets... At least I can confirm dbd-sqlite3 stopped working in Ruby 2.0.
  • Kernel#zip Perhaps I should add a testcase: zip([[1,2,3],[4,5]]).should eq [[1, 4],[2, 5],[3, nil]]. [[1,2,3],[4,5]].transpose fails. Well, here, [1,2,3].zip([4,5]) can the same thing. So it doesn't need to be included...
  • Array#permutation2 [1,1,2,3].permutation2 is different from [1,1,2,3].permutation.
  • Hash#patch You are correct. Removed in 0.0.0.3.
  • Enumerator::Lazy#squeze Thank you. By the way if we unsupport enumerable/lazy and force backports gem, Lazy.class_eval can be changed to class Lazy, which is cleaner. How would you like?
  • Hash#fetch_nested Your implementation is brilliant...

libciel was updated to 0.0.0.3. Now source structure should be OK. Next task should be facets-style documentation...

posted by cielavenir almost 11 years ago

The best way to go about this, if you have the time and you'd really like to see these methods make into Facets, is to fork the Facets repo, then add one method at a time, with documentation, Lemon unit test and QED demo, and then commit and send a pull request. That way we can evaluate/discuss/merge each method individually on its own merits.

posted by trans almost 11 years ago

We have Array.zip class method, instead of Kernel#zip. I can see how Kernel#zip can be convenient, but the same can be said about many Enumerable and Array methods. In such cases, it's best for the developer to define a private method where they want the convenience, e.g.

    def zip(*a); Array.zip(*a); end
posted by trans almost 11 years ago

For the DBI extensions, I would suggest submitting them to the DBI project (I assume that's the official project). Facets isn't the right place for those --it is all about Ruby's core and standard libraries.

Note, I just read the the first issue in the DBI project and it appears the project is no longer being maintained, so there is an opportunity there, if you are a user, to bring your own influence to the project.

posted by trans almost 11 years ago

I handed in my master thesis...

Very sorry but I'm not sure if I'm doing the right thing.

#fork facets repo
git clone git@github.com:cielavenir/facets.git
cd facets
sudo bundle
lemons test

produces:

/Library/Ruby/Gems/2.0.0/gems/lemon-0.9.1/lib/lemon/cli.rb:28:in `cli': undefined method `cli' for Test::Runner:Class (NoMethodError)
    from /Library/Ruby/Gems/2.0.0/gems/lemon-0.9.1/bin/lemons:8:in `<top (required)>'
    from /usr/bin/lemons:23:in `load'
    from /usr/bin/lemons:23:in `<main>'
posted by cielavenir almost 11 years ago

This will be fixed via https://github.com/rubyworks/facets/pull/86. Now I'm going to do branching.

posted by cielavenir almost 11 years ago

I haven't tried the lemons command in some time. I'll have to look at that. But it's a thin wrapper around rubytest, so you can use that instead. However, it's much easier just to run rake test.

posted by trans almost 11 years ago

@boostio funded this issue with $10. Visit this issue on Issuehunt

posted by IssueHuntBot over 6 years ago

It seems the proposed methods already exist in the code base?

posted by DaHoopster about 6 years ago

Fund this Issue

$10.00
Funded
Only logged in users can fund an issue

Pull requests

Recent activities

boostio funded 10.00 for rubyworks/facets# 80
over 6 years ago