search

PyAOS / aoslib

A collection of standard atmospheric and oceanic sciences routines.
45 stars 32 forks source link

Added wrappers for additional functions #16

Closed jjhelmus closed 10 years ago

jjhelmus commented 10 years ago

Added wrappers around the following FORTRAN functions

jjhelmus commented 10 years ago

I plan on writing one wrapper a day for the next few weeks, I will add them to this Pull Request, please don't merge until a number of wrappers have build up (I'll let you know).

rhattersley commented 10 years ago

please don't merge until a number of wrappers have build up (I'll let you know).

I recommend you don't leave it too long though. Little and often is easier to review.

jwblin commented 10 years ago

Okay, thanks for the heads-up! (And, of course, for the wrapping!)

jjhelmus commented 10 years ago

6 functions wrapped this past week. Probably good to have these reviewed and merged. I'll start a new branch for the weeks work.

jwblin commented 10 years ago

Okay, will do so soon. I'll also ask others to do some review. Btw, what happens if you get multiple pull requests? Assuming that each request edits different parts of the same files, can you merge them in any order? I would assume that would be the case, but I just wanted to ask. Thanks!

On Mon, Feb 17, 2014 at 08:35:47AM -0800, Jonathan J. Helmus wrote:

6 functions wrapped this past week. Probably good to have these reviewed and merged. I'll start a new branch for the weeks work.

Reply to this email directly or view it on GitHub:

https://github.com/PyAOS/aoslib/pull/16#issuecomment-35300252

Johnny Wei-Bing Lin Professor of Physics

North Park University (773) 244-6266 phone johnny@johnny-lin.com www.johnny-lin.com

jjhelmus commented 10 years ago

When you need merge multiple git branches you have two options. The easiest is to merge each branch one-by-one independently. If the branches do not conflict (as in the don't both change the same lines/files) then the merging is simple and you should be able to do it through GitHub. When the branches conflict then someone will need to resolve the conflict. The Pro Git book has a good section on this. I don't believe GitHub offers an interface to resolve conflicts so it would need to be done manually in a shell/GUI (I've done this a few times it is not too difficult).

The other choice is to try to merge all the branches at the same time in what is called a "octopus" merge. This is more difficult and I my opinion should be avoided unless there is a truly compelling case not to do the merging one-by-one.

jwblin commented 10 years ago

Thanks!

On Tue, Feb 18, 2014 at 08:03:22AM -0800, Jonathan J. Helmus wrote:

When you need merge multiple git branches you have two options. The easiest is to merge each branch one-by-one independently. If the branches do not conflict (as in the don't both change the same lines/files) then the merging is simple and you should be able to do it through GitHub. When the branches conflict then someone will need to resolve the conflict. The Pro Git book has a good section on this. I don't believe GitHub offers an interface to resolve conflicts so it would need to be done manually in a shell/GUI (I've done this a few times it is not too difficult).

The other choice is to try to merge all the branches at the same time in what is called a "octopus" merge. This is more difficult and I my opinion should be avoided unless there is a truly compelling case not to do the merging one-by-one.

Reply to this email directly or view it on GitHub:

https://github.com/PyAOS/aoslib/pull/16#issuecomment-35399149

Johnny Wei-Bing Lin Professor of Physics

North Park University (773) 244-6266 phone johnny@johnny-lin.com www.johnny-lin.com

jwblin commented 10 years ago

Jonathan H., thanks much for your patience! I thought I'd use this pull request to get more folks involved in the code review. Dave has agreed to review this request and I've added him as admin so he can do the merge once he's done the review.

david-ian-brown commented 10 years ago

Jonathan, The code and the tests looked fine to me. I made a few in-line comments about the documentary lines in the wrapper file. Sorry for the delay in reviewing your work.

jjhelmus commented 10 years ago

@lizard-spring I think I addressed all the commend. The syntax of the dgeocomps function is still a bit unwieldy with the differences between Python and Fortan rearing its head. Long term I think it would be easier to rewrite the function in pure Python (or Cython) then try to fix the syntax problems in the Fortan interface.

jwblin commented 10 years ago

Hi Dave,

When you're ready, just go ahead and do the merge. You should have permission to do so. Thanks!

Best, -Johnny

david-ian-brown commented 10 years ago

OK, yes I'm planning to do that today. -dave

On Mar 11, 2014, at 10:59 AM, Johnny Lin notifications@github.com wrote:

Hi Dave,

When you're ready, just go ahead and do the merge. You should have permission to do so. Thanks!

Best, -Johnny

— Reply to this email directly or view it on GitHub.

david-ian-brown commented 10 years ago

Jonathan's code is merged, I think successfully. Let me know if anything does not seem right. -dave

On Mar 11, 2014, at 12:19 PM, David Brown dbrown@ucar.edu wrote:

OK, yes I'm planning to do that today. -dave

On Mar 11, 2014, at 10:59 AM, Johnny Lin notifications@github.com wrote:

Hi Dave,

When you're ready, just go ahead and do the merge. You should have permission to do so. Thanks!

Best, -Johnny

— Reply to this email directly or view it on GitHub.

jwblin commented 10 years ago

Looks good! Thanks!

On Tue, Mar 11, 2014 at 12:02:53PM -0700, Dave Brown wrote:

Jonathan's code is merged, I think successfully. Let me know if anything does not seem right. -dave

On Mar 11, 2014, at 12:19 PM, David Brown dbrown@ucar.edu wrote:

OK, yes I'm planning to do that today. -dave

On Mar 11, 2014, at 10:59 AM, Johnny Lin notifications@github.com wrote:

Hi Dave,

When you're ready, just go ahead and do the merge. You should have permission to do so. Thanks!

Best, -Johnny

— Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub:

https://github.com/PyAOS/aoslib/pull/16#issuecomment-37336191

Johnny Wei-Bing Lin Professor of Physics

North Park University (773) 244-6266 phone johnny@johnny-lin.com www.johnny-lin.com

jjhelmus commented 10 years ago

Dave, thanks for the review. I'll have some addition wrappers in a week or two.

david-ian-brown commented 10 years ago

You bet. And thank you for all your work. -dave

On Mar 12, 2014, at 8:29 AM, "Jonathan J. Helmus" notifications@github.com wrote:

Dave, thanks for the review. I'll have some addition wrappers in a week or two.

— Reply to this email directly or view it on GitHub.