search

cyjoelchen / php-sweph

PHP Extension for Swiss Ephemeris
Other
63 stars 29 forks source link

Add documentation & test for swe_deltat(). #37

Closed kevindecapite closed 3 years ago

kevindecapite commented 3 years ago

Refs: https://github.com/cyjoelchen/php-sweph/issues/36

If I don't finish this before I need to leave this afternoon, I will return to it in the evening. If you feel like picking up where I left off, let us simply append our commits to this branch (issue/36) until it's completed then we can review and merge into master.

aloistr commented 3 years ago

I don't understand the review process. What am I supposed to click if I agree? I only see "convert to draft" and do not understand what that means in the context.

kevindecapite commented 3 years ago

Screen Shot 2021-07-19 at 9 52 23 PM

kevindecapite commented 3 years ago

I'm going to continue to add the remaining docs/tests in this PR one by one. So I will request your review again once they have been added.

kevindecapite commented 3 years ago

Makes sense. I will handle that change separately since that is an interface change to the PHP implementation itself.

https://github.com/cyjoelchen/php-sweph/issues/39

kevindecapite commented 3 years ago

By the way, you do not need to approve each commit. Typically a review takes place once all the work is completed, which may not happen this evening as it's getting late for me.

aloistr commented 3 years ago

my new test swe_lun_occult.phpt crashes wit this output swe_lun_occult_when_glob for Venus

Termsig=11

It seems to happen in if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "dlslll", &tjd_start, &ipl, &starname, s_len, &ifl, &ifltype, &backward, &arg_len) == FAILURE) {

I do currently not even call C function.

kevindecapite commented 3 years ago

One thing I noticed is that ipl, ifl, ifltype and backward are all declared asint, but are expected to be long in zend_parse_parameters(). However, given that the former is smaller than the latter, I wouldn't expect this to be a problem. I am not a c programmer, however, and am referencing the Sams book on c programming as a guide, while making some assumptions.

Also, I did change those vars to long but still received "Segmentation fault" when testing.

aloistr commented 3 years ago

now I see the bug. mustbe &tjd_start, &ipl, &starname, &s_len, &ifl, &ifltype,

aloistr commented 3 years ago

I think we should merge to master!

aloistr commented 3 years ago

All the undocumented eclipse and occultation functions are still likely to be buggy, but I have limited time to fix them now.

kevindecapite commented 3 years ago

I think we should merge to master!

Done. I will continue writing PODS and do what I can to fix what may be broken in the remaining functions.

aloistr commented 3 years ago

I will fix and document the eclipse functions.