Handling adhika masas for festivals

[karthikraman]

Need to expand the toml description to include a tag for adhika masas

There are festivals that are celebrated

• only in adhika masa
• in adhika masa alone, if adhika masa exists or in nija masa otherwise
• in both adhika and nija masas
• only in nija masa

The above logic should be supported by the toml and encode in the decision making.

[vvasuki]

Duplicate or related to https://github.com/jyotisham/jyotisha/issues/80 ?

[karthikraman]

Good point. I had forgotten #80. And #80 was somewhat simpler, as it may not permit things like point 2 above... I think this should supersede #80.

[karthikraman]

This would require changing the toml schema, right? Thoughts?

[vvasuki]

Could add a field under timing:

adhika_maasa_handling = "adhika_only"
adhika_maasa_handling = "adhika_if_exists"
adhika_maasa_handling = "adhika_and_nija"
adhika_maasa_handling = "nija_only"

Default if unspecified would be what?

[karthikraman]

nija_only

[karthikraman]

@vvasuki this has been truly challenging, partly because I haven't fully understood the festival calculation perfectly. Can you eyeball the last few commits, and suggest ways to improve? What would be the best place to implement the handling?

1. While loading .toml files, we could process and add additional festivals (I tried it and commented it out)
2. Modify get_possibly_relevant_fests() as I have mostly done here
3. Modify _should_assign_festival() as I have done here - I seem to not be able to get around this exception check and had to hack together more exception handling here.

What do you think is the best way forward? I should have really started this in a separate branch -- totally underestimated the difficulty of the problem. All tests pass currently (but I think some adhika maasa festivals like ArambhaH and samApanam are not assigned)

[vvasuki]

The tests were failing on my computer. Fixed those, simplified the code, added comments. You should be set for further changes.

You were roughly on the right track - get_possibly_relevant_fests needed to be modified. Now, get_possibly_relevant_fests was not properly fixed/ tested; so you needed to add ~80 lines downstream in apply_month_anga_events to handle edge cases that slipped through (removed all that junk).

Things I noted:

• If you insert 80 lines of code into a (previously short) function, you're severely impeding readability and that leads to errors you won't notice. If necessary, add and call new functions.
• No unit tests. If you had written a thorough unit test (<- still doesn't exist) for get_possibly_relevant_fests, you would've had much smoother sailing.

Good luck forward!