Closed kevindecapite closed 3 years ago
The test does not set ephepath, so it defaults to the old internal default path ".:/users/ephe:/users/ephe2"
I have the files in this path, you probably don't
That exposes a general problem for tests. The internally referenced ephemeris files must exist and be found in the test environment.
Am 18.07.2021 um 09:13 schrieb Kevin DeCapite @.***>:
@kevindecapite commented on this pull request.
In tests/swe_fixstar.phpt:
["rc"]=>
- int(-1)
- int(4) How were we getting different results running under the same conditions?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
Having a copy of file sefstars.txt in directory php-sweph solves the issue, on Linux at least. The default path contains the dot directory .
We need files seas_18.se1 sefstars.txt semo_18.se1 sepl_18.se1 to make swe_calc test use Swiss Ephemeris mode and and Moshier mode.
Am 18.07.2021 um 09:20 schrieb @.***:
The test does not set ephepath, so it defaults to the old internal default path ".:/users/ephe:/users/ephe2"
I have the files in this path, you probably don't
That exposes a general problem for tests. The internally referenced ephemeris files must exist and be found in the test environment.
Am 18.07.2021 um 09:13 schrieb Kevin DeCapite @.***>:
@kevindecapite commented on this pull request.
In tests/swe_fixstar.phpt:
["rc"]=>
- int(-1)
- int(4) How were we getting different results running under the same conditions?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
The internally referenced ephemeris files must exist and be found in the test environment.
I thought about including them in this repo but considered it might be acceptable to allow the function to fail as I originally had it since these tests only need to verify that the SE methods are implemented correctly.
I am open to the idea of including a minimal set of the data files in this repo, however.
I have added the necessary 4 files seas_18.se1 sefstars.txt semo_18.se1 sepl_18.se1
Test cases should have dates in the year range 1800 - 2399. Moshier mode is deprecated, it will go in future releases. Built in binary ephemeris files will replace it, to support single file installations.
Am 18.07.2021 um 09:32 schrieb Kevin DeCapite @.***>:
The internally referenced ephemeris files must exist and be found in the test environment.
I thought about including them in this repo but considered it might be acceptable to allow the function to fail as I originally had it since these tests only need to verify that the SE methods are implemented correctly.
I am open to the idea of including a minimal set of the data files in this repo, however.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
float or double Kevin, you converted my 'double' in pods to 'float'
I suggest to stick to 'double'. I know that the formal name of floating point number in PHP is 'float', but on many places it is said "Floating point numbers (also known as "floats", "doubles", or "real numbers")"
We are dealing with an API to a C library here, and in C "float" means reduced precision 4-byte real number, "double" means ful precision 8-byte real number. As a C programmer it confuses me to see 'float'.
As double is widely accepted as equivalent to float, in PHP, I suggest we use 'double' in the docu.
I suggest to stick to 'double'.
I did expect that change to not go unnoticed. I was keeping with common PHP syntax as rarely, if ever, will you see type definitions as double
. I am fine to use that term, however, as it's more correct from a broader technological standpoint (i.e. outside the PHP community).
I only meant to not confuse the average PHP developer with it.
Test tests/swe_get_library_path.phpt wil always fail for me, and probably for others too.
my diff is 001+ string(38) "/home/alois/php-sweph/modules/sweph.so" 001- string(32) "/root/php-sweph/modules/sweph.so" and it will never succeed, because not all users will install php-sweph in /root
I suggest the test should only check that "/php-sweph/modules/sweph.so" is contained in the reply.
Test tests/swe_get_library_path.phpt wil always fail for me, and probably for others too.
Thank you for pointing that out. I changed the test to use a substring of the entire path (/modules/sweph.so
) so this should pass in all environments now.
@aloistr
I deleted this branch now that it's merged. This is a typical part of the workflow. I am leaving the issue open, however, as there is more to do still.
To update and clean your local environment:
> git fetch
> git check master
> git pull origin master
> git remote prune origin
I like to remove my local copies of deleted remote branches with:
> git branch -d name_of_branch
Refs:
I am adding documentation one function at a time, using the POD method prescribed by @aloistr and simultaneously adding a test for each function as well.
This PR is a WIP (work-in-progress) and should not be merged until all current methods are documented and tested.
Collaboration is welcome.