jyotisham / jyotisha

Python tools for the astronomical / astrological vedAnga of Hindus
MIT License
88 stars 53 forks source link

NAMES dict: undo quick&dirty fixes for zero indexing #14

Closed vvasuki closed 6 years ago

vvasuki commented 6 years ago

mitra, With https://github.com/sanskrit-coders/jyotisha/commit/145256122a4e6e3fb2319db5e9296fb1a5f79834 and https://github.com/sanskrit-coders/jyotisha/commit/862beef2a8d32fe32e4c9071495f121f93eda052 , I thought I had fixed the zero indexing problems caused by removing custom parsing code. But I now realize that I had missed some, looking at https://github.com/sanskrit-coders/jyotisha/commit/149a2d9c12369947820d4ac5532458656f02ca36 . It would've been better to fix the indexing in those files which I'd missed. Could you replace this quick n dirty fix with proper indexing?

karthikraman commented 6 years ago

Yes, will do that for sure. This was to get the code working right -- and now I can do incremental fixes without breaking...

karthikraman commented 6 years ago

Not sure if it's a good idea to index with zero. It will demand ugly -1s all over the code. Instead, I think this blank first element (or "None"?) would give us cleaner code. Alternatively, we could use a dictionary instead of an array, indexing with 1, 2, ....

vvasuki commented 6 years ago

Indexing from zero is not really ugly - it's standard in computer science.. Or atleast no longer ugly given the all these decades of computer science tradition (like those kavisamaya-s which say that a bee is trapped in a padma at dusk).. Still - if necessary, you could write a wrapper function that does the -1 for you..

karthikraman commented 6 years ago

Yes, zero indexing per se is not ugly, but -1 all over the codes will be :). Alternatively, Ashwini has to be Nakshatra Zero, Shukla Prathama has to be tithi 0 (Amavasya would make more sense) -- this will require major code re-write, which I plan to do anyway, to completely decouple the calculation and TeXing etc. as described in issue #7

karthikraman commented 6 years ago

Unfortunately, festival_rules.json etc. rely on 1-27 numbering for Nakshatras etc. So, a careful fix is too hard at the moment. The commit 66a702c provides a middle ground, for now, I believe! Keeping the issue alive, nevertheless, for a better fix in the future.

vvasuki commented 6 years ago

Better! Improved with https://github.com/sanskrit-coders/jyotisha/commit/424160db7ab016c9bd2b6a1b4e1f905426204b72 . Ideal would be to throw an exception at index 0, which is possible only with a wrapper.