www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.bitmanip improvements

reply "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
Hi,
I was looking into the BitArray part of std.bitmanip to implement 
various data compression routines like Huffmann, Lempel-Ziv etc.. 
and noticed that it could use some improvements, like more 
convenient constructors and functions (for ex. see issue 4717 ). 
How would I go about submitting those changes? Should I put all 
the changes in one big pull request or split it up according to 
the different enhancement requests on Bugzilla?
Also is anyone else working on this module or is there something 
in other parts of the library that I should be aware of?

Cheers,
Marco
Dec 27 2012
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Marco Nembrini:


 Also is anyone else working on this module or


Recently I remember some discussions in D.learn about someone 
working on that array of bits.

Bye,
bearophile
Dec 27 2012
prev sibling next sibling parent "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Marco Nembrini" <marco.nembrini.co gmail.com> wrote in message 
news:ezcalrajgcqlspefnlri forum.dlang.org...

 Should I put all the changes in one big pull request or split it up 
 according to the different enhancement requests on Bugzilla?


Split it up.
Dec 27 2012
prev sibling next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
12/27/2012 1:38 PM, Marco Nembrini пишет:

 Hi,
 I was looking into the BitArray part of std.bitmanip to implement
 various data compression routines like Huffmann, Lempel-Ziv etc.. and
 noticed that it could use some improvements, like more convenient
 constructors and functions (for ex. see issue 4717 ). How would I go
 about submitting those changes? Should I put all the changes in one big
 pull request or split it up according to the different enhancement
 requests on Bugzilla?


Judging by my experience small pulls that have no extra dependencies on 
other pulls are the fastest to get merged. The are also more throughly 
reviewed :) So I'd suggest do it step by step in a couple of pulls if 
possible.



 Also is anyone else working on this module or is there something in
 other parts of the library that I should be aware of?


IRC Era Scarecrow did an amazing job on it that covered a lot of 
enhancements including small string optimization, appending, slicing etc.

I recall reviewing his code and it was a Phobos pull once but then it 
pulled out (for further enhancement? dunno). So definitely suggest you 
two to get in touch.


 Cheers,
 Marco



-- 
Dmitry Olshansky
Dec 27 2012
next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:

 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.


Last time I was in touch with him, he was getting increasingly 
frustrated trying to learn to use github. I *think* (but don't 
take my word for it), that he may have abandoned the pull after 
having too much trouble trying to rebase it.

In any case, I'll try to contact him directly and make him aware 
of this thread.
Dec 27 2012
prev sibling next sibling parent reply "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:

 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.


  I'm glad for the praise... :)

  Perhaps it seems silly, however I pulled it not because I needed 
to enhance it, but because it had trouble merging with other 
pulls. I looked at the error(s) and it was something like '13 
white spaces mismatches'... How do you fix 13 white spaces? 
They're white space because you can't see them :P Had it been 
lines of code you can intelligently re-order or adjust them, but 
white spaces?? Sheesh...

  Maybe being lazy but it was a headache trying to figure out the 
diff patch and how to fix it, so haven't tried too hard on it 
yet. If someone happens across that I'll give it another go and 
see if there's any other fixes/enhancements I can throw on it; 
Maybe get it in before the next release.
Dec 27 2012
parent reply Marco Nembrini <marco.nembrini.co gmail.com> writes:
On 28.12.2012 11:48, Era Scarecrow wrote:

 On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky wrote:

 IRC Era Scarecrow did an amazing job on it that covered a lot of
 enhancements including small string optimization, appending, slicing etc.

 I recall reviewing his code and it was a Phobos pull once but then it
 pulled out (for further enhancement? dunno). So definitely suggest you
 two to get in touch.


 I'm glad for the praise... :)

 Perhaps it seems silly, however I pulled it not because I needed to
 enhance it, but because it had trouble merging with other pulls. I
 looked at the error(s) and it was something like '13 white spaces
 mismatches'... How do you fix 13 white spaces? They're white space
 because you can't see them :P Had it been lines of code you can
 intelligently re-order or adjust them, but white spaces?? Sheesh...

 Maybe being lazy but it was a headache trying to figure out the diff
 patch and how to fix it, so haven't tried too hard on it yet. If someone
 happens across that I'll give it another go and see if there's any other
 fixes/enhancements I can throw on it; Maybe get it in before the next
 release.


So the code was complete and working and the only problem was doing the 
pull on github?
If so would you mind me trying to do that in your place? It seems worth 
spending some time on to get all those improvements in.

Is the latest version this one: https://github.com/rtcvb32/phobos ?
Dec 27 2012
next sibling parent "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:

 So the code was complete and working and the only problem was 
 doing the pull on github? If so would you mind me trying to do 
 that in your place? It seems worth spending some time on to get 
 all those improvements in.


  In a nutshell, yes..


 Is the latest version this one: 
 https://github.com/rtcvb32/phobos ?


  I'll have to look at my sources, I may have been working on 
something else. I can't recall but I think the settings were 
'sometimes reference', although that's not hard to fix, just need 
to to decide the best way to handle the data that way.

  I'll look over all my stuff tomorrow. Feel free to try and get 
it to merge/pull in GitHub.
Dec 27 2012
prev sibling parent reply "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:

 If so would you mind me trying to do that in your place? It 
 seems worth spending some time on to get all those improvements 
 in.


  Well since this is going I've resumed some work on it, nice to 
see the opBinary works along with the opDollar. I seem to have 
problems figuring out where to tell my github source to get the 
current beta so I can just work/update from that. All of it is 
quite confusing to me.
Jan 01 2013
parent Marco Nembrini <marco.nembrini.co gmail.com> writes:
On 01.01.2013 19:14, Era Scarecrow wrote:

 On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:

 If so would you mind me trying to do that in your place? It seems
 worth spending some time on to get all those improvements in.


   Well since this is going I've resumed some work on it, nice to see the
 opBinary works along with the opDollar. I seem to have problems figuring
 out where to tell my github source to get the current beta so I can just
 work/update from that. All of it is quite confusing to me.


Checkout the section about rebasing here: 
http://wiki.dlang.org/Pull_Requests . If you still have trouble send me 
a message, I should be able to help with git at least.

Cheers

-- 

Marco Nembrini
Jan 01 2013
prev sibling parent reply "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:

 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.


I assume you are referring to the code in this thread: 
http://forum.dlang.org/thread/gtfxxmlqwgyadmpygftv forum.dlang.org 
?
Dec 27 2012
parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
12/28/2012 5:13 AM, Marco Nembrini пишет:

 On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky wrote:

 IRC Era Scarecrow did an amazing job on it that covered a lot of
 enhancements including small string optimization, appending, slicing etc.

 I recall reviewing his code and it was a Phobos pull once but then it
 pulled out (for further enhancement? dunno). So definitely suggest you
 two to get in touch.


 I assume you are referring to the code in this thread:
 http://forum.dlang.org/thread/gtfxxmlqwgyadmpygftv forum.dlang.org ?


Yes, this one.

-- 
Dmitry Olshansky
Dec 27 2012
prev sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, December 27, 2012 10:38:35 Marco Nembrini wrote:

 Hi,
 I was looking into the BitArray part of std.bitmanip to implement
 various data compression routines like Huffmann, Lempel-Ziv etc..
 and noticed that it could use some improvements, like more
 convenient constructors and functions (for ex. see issue 4717 ).
 How would I go about submitting those changes? Should I put all
 the changes in one big pull request or split it up according to
 the different enhancement requests on Bugzilla?
 Also is anyone else working on this module or is there something
 in other parts of the library that I should be aware of?


I believe that there are a couple of threads from a few months ago in D.Learn 
talking about improvements to BitArray, and some folks having been working on 
that. I have no idea where that stands though.

In general, if you have incremental improvements for things, feel free to 
create pull requests for them on github. Unrelated requests should be 
separate. Please don't make huge pulls with lots of stuff in them. It makes 
reviewing them harder, and it's more problematic when some of your changes are 
acceptable and others aren't. If they're separate, they're much more easily 
managed.

However, if you're talking about seriously revamping the module as a whole 
rather than making a number of small improvements, then that may merit a full 
review in the newsgroup, which is a different beast altogether.

- Jonathan M Davis
Dec 27 2012
parent "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
On Thursday, 27 December 2012 at 10:21:37 UTC, Jonathan M Davis 
wrote:

 I believe that there are a couple of threads from a few months 
 ago in D.Learn
 talking about improvements to BitArray, and some folks having 
 been working on
 that. I have no idea where that stands though.


Ok, I'll try to read up on that and get in touch with others 
involved.


 In general, if you have incremental improvements for things, 
 feel free to
 create pull requests for them on github. Unrelated requests 
 should be
 separate. Please don't make huge pulls with lots of stuff in 
 them. It makes
 reviewing them harder, and it's more problematic when some of 
 your changes are
 acceptable and others aren't. If they're separate, they're much 
 more easily
 managed.

 However, if you're talking about seriously revamping the module 
 as a whole
 rather than making a number of small improvements, then that 
 may merit a full
 review in the newsgroup, which is a different beast altogether.

 - Jonathan M Davis


I was thinking of mostly small convenience changes, I don't feel 
familiar enough with D yet to do a revamp of that, but maybe I 
can help get Era Scarecrow's version pulled.
Dec 27 2012