lballabio / quantlib-old

The QuantLib C++ library and extensions (warning: out of date)
http://quantlib.org
471 stars 405 forks source link

Update and rename QuantLib/ql/time/calendars/saudiarabia.cpp to Quant… #360

Closed jrvarma closed 8 years ago

jrvarma commented 8 years ago

…Lib34/ql35/time36/calendars37/saudiarabia.cpp

The Saudi Arabian (Tadawul Exchange) calendar in QuantLib has not been updated since 2011 and does not reflect the change in the weekend (from Thu/Fri to Fri/Sat) that was made in 2013. Also, the data on the Eid holidays (which varies from year to year) are available sparsely for 2004-2011 only.

This pull request is an attempt to improve the QuantLib implementation of the Saudi calendar. It implements the change in the weekend and also populates the Eid dates for about 30 years using public sources. This is still imperfect as it assumes that the Eid holidays are for a fixed number of days around the actual Eid dates, though in practice the duration of the holidays also varies. Also one-off holidays are ignored.

The Saudi calendar is not of much financial importance, but I find it very useful for educational purposes. I use it as a reminder to my students that even the most trivial assumptions (like weekend is Sat/Sun) should not be taken for granted. Hence an updated Saudi calendar even if imperfect would be useful.

I am not too familiar with C++ coding conventions used in QuantLib. My apologies if I have violated these coding guidelines. (I work mainly with QuantLib-Python).

lballabio commented 8 years ago

Thanks, but we'll need some order if we're to be able to merge this pull request. First: please don't rename files. We have version control to track changes :smile: Renaming the files defeats the purpose and makes it more difficult to merge your changes. Second: you created the pull request from your master. Unfortunately, this means that whatever you commit and push to your master is ending in the PR, too. It's probably better if you move the calendar change to a branch and recreate the PR from there.

Sorry for stopping you in your tracks---I do appreciate the contribution---but this risks getting difficult to manage very quickly...

jrvarma commented 8 years ago

On Mon, Nov 23 2015 at 08:03:10 PM, Luigi Ballabio notifications@github.com wrote:

Thanks, but we'll need some order if we're to be able to merge this pull request. First: please don't rename files. We have version control to track changes :smile: Renaming the files defeats the purpose and makes it more difficult to merge your changes. Second: you created the pull request from your master. Unfortunately, this means that whatever you commit and push to your master is ending in the PR, too. It's probably better if you move the calendar change to a branch and recreate the PR from there. Sorry for all this mess. I do not have much experience with this.

Am still not able to make this pull request work correctly after couple of attempts. Will need to read up more about this process

over the next few days.


Prof. Jayanth R. Varma, Indian Institute of Management, Vastrapur, Ahmedabad 380 015 INDIA email: jrvarma@iimahd.ernet.in OR jrvarma@gmail.com Tel: +91-79-6632 4867 (Off) +91-79-6632 5318 (Res) Personal Homepage: http://www.iimahd.ernet.in/~jrvarma/ Personal Blog: http://www.iimahd.ernet.in/~jrvarma/blog


lballabio commented 8 years ago

Ok, no problem. Whistle if you need help.

jrvarma commented 8 years ago

On Mon, Nov 23 2015 at 08:03:10 PM IST, Luigi Ballabio notifications@github.com wrote:

Thanks, but we'll need some order if we're to be able to merge this pull request.

I hope that I have got things right this time around with the two pull requests of today. If you can delete the old pull request of yesterday, you may please do so.

First: please don't rename files. We have version control to track changes :smile: Renaming the files defeats the purpose and makes it more difficult to merge your changes.

There was actually no file rename. What happened was that I did not compose a commit message and then GitHub automatically generates this message about renaming files though the file name is not changed -- there are only some numbers 34/35/36 in the folder names presumably to identify my fork. Anyway with a proper commit message, things should be OK now. Githib still shows two files as changed and still has these numbers in my file. If I am still making a mistake, please let me know. I have read the GitHub guides and they do not explain where these mysterious numbers come from and whether they are harmless.

Second: you created the pull request from your master. Unfortunately, this means that whatever you commit and push to your master is ending in the PR, too. It's probably better if you move the calendar change to a branch and recreate the PR from there.

This time, I have pull from non master branches.


Prof. Jayanth R. Varma, Indian Institute of Management, Vastrapur, Ahmedabad 380 015 INDIA email: jrvarma@iimahd.ernet.in OR jrvarma@gmail.com Tel: +91-79-6632 4867 (Off) +91-79-6632 5318 (Res) Personal Homepage: http://www.iimahd.ernet.in/~jrvarma/ Personal Blog: http://www.iimahd.ernet.in/~jrvarma/blog


lballabio commented 8 years ago

No, that's an actual file rename. If I pull your request, I get an actual new file QuantLib34/ql35/time36/calendars37/saudiarabia.cpp. Are you sure you don't have it on your machine?

jrvarma commented 8 years ago

On Tue, Nov 24 2015 at 05:26:29 PM IST, Luigi Ballabio notifications@github.com wrote:

No, that's an actual file rename. If I pull your request, I get an actual new file QuantLib34/ql35/time36/calendars37/saudiarabia.cpp. Are you sure you don't have it on your machine? My machine does not come into the picture because I am using the GitHub web interface to edit the file. I did not push from my machine because I am running an older version of QuantLib to test

my code.


Prof. Jayanth R. Varma, Indian Institute of Management, Vastrapur, Ahmedabad 380 015 INDIA email: jrvarma@iimahd.ernet.in OR jrvarma@gmail.com Tel: +91-79-6632 4867 (Off) +91-79-6632 5318 (Res) Personal Homepage: http://www.iimahd.ernet.in/~jrvarma/ Personal Blog: http://www.iimahd.ernet.in/~jrvarma/blog


lballabio commented 8 years ago

That's weird. Anyway: pushing from an older version is not a problem; git can merge your changes into the current code. (Actually, pasting from a modified old version on top of a newer one can cause problems, instead, since the old code might overwrite modifications made in the meantime.)