search

sot / chandra_time

Convert between various time formats relevant to Chandra.
https://sot.github.io/Chandra.Time
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Change Chandra.Time to chandra_time w/ back-compatibility #54

Closed taldcroft closed 1 year ago

taldcroft commented 1 year ago

Description

Basically the same as https://github.com/sot/Ska.Sun/pull/22.

The only difference here is that this package has a cython compiled package so that needs to be handled. I was unable to find the incantation that would get the cython-compiled module to build and install in Chandra.Time, but importing from chandra_time._axTime3 should be fine since the chandra_time package always built and installed alongside Chandra.Time.

This PR also makes a few minor updates to the docs, in particular a note that DateTime is deprecated in favor of CxoTime.

Requires https://github.com/sot/testr/pull/46.

Interface impacts

Testing

Unit tests

Independent check of unit tests by [REVIEWER NAME]

Functional tests

Build

(ska3-dev) ➜  Chandra.Time git:(de-namespace) ✗ python setup.py install --single-version-externally-managed --record=record.txt
running install
running build
running build_py
creating build
creating build/lib.macosx-10.9-x86_64-3.8
creating build/lib.macosx-10.9-x86_64-3.8/chandra_time
copying chandra_time/Time.py -> build/lib.macosx-10.9-x86_64-3.8/chandra_time
copying chandra_time/__init__.py -> build/lib.macosx-10.9-x86_64-3.8/chandra_time
copying chandra_time/axTime3.py -> build/lib.macosx-10.9-x86_64-3.8/chandra_time
creating build/lib.macosx-10.9-x86_64-3.8/chandra_time/tests
copying chandra_time/tests/test_Time.py -> build/lib.macosx-10.9-x86_64-3.8/chandra_time/tests
copying chandra_time/tests/__init__.py -> build/lib.macosx-10.9-x86_64-3.8/chandra_time/tests
creating build/lib.macosx-10.9-x86_64-3.8/Chandra
creating build/lib.macosx-10.9-x86_64-3.8/Chandra/Time
copying chandra_time/Time.py -> build/lib.macosx-10.9-x86_64-3.8/Chandra/Time
copying chandra_time/__init__.py -> build/lib.macosx-10.9-x86_64-3.8/Chandra/Time
copying chandra_time/axTime3.py -> build/lib.macosx-10.9-x86_64-3.8/Chandra/Time
creating build/lib.macosx-10.9-x86_64-3.8/Chandra/Time/tests
copying chandra_time/tests/test_Time.py -> build/lib.macosx-10.9-x86_64-3.8/Chandra/Time/tests
copying chandra_time/tests/__init__.py -> build/lib.macosx-10.9-x86_64-3.8/Chandra/Time/tests
running build_ext
building 'chandra_time._axTime3' extension
creating build/temp.macosx-10.9-x86_64-3.8
creating build/temp.macosx-10.9-x86_64-3.8/chandra_time
gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -Ichandra_time -I/Users/aldcroft/miniconda3/envs/ska3-dev/include/python3.8 -c chandra_time/_axTime3.cpp -o build/temp.macosx-10.9-x86_64-3.8/chandra_time/_axTime3.o -Wno-switch-enum -Wno-switch -Wno-switch-default -Wno-deprecated -Wno-parentheses -stdlib=libc++
gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -Ichandra_time -I/Users/aldcroft/miniconda3/envs/ska3-dev/include/python3.8 -c chandra_time/axTime3.cc -o build/temp.macosx-10.9-x86_64-3.8/chandra_time/axTime3.o -Wno-switch-enum -Wno-switch -Wno-switch-default -Wno-deprecated -Wno-parentheses -stdlib=libc++
chandra_time/axTime3.cc:380:11: warning: unused variable 'fgets_value' [-Wunused-variable]
    char* fgets_value = fgets (tform, 10, stdin) ;
          ^
chandra_time/axTime3.cc:56:26: warning: unused variable 'rcsid' [-Wunused-const-variable]
static const char* const rcsid = "axTime $Id: axTime3.cc,v 1.2 2007-08-27 21:30:59 aldcroft Exp $" ;
                         ^
2 warnings generated.
gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -I/Users/aldcroft/miniconda3/envs/ska3-dev/include -arch x86_64 -Ichandra_time -I/Users/aldcroft/miniconda3/envs/ska3-dev/include/python3.8 -c chandra_time/XTime.cc -o build/temp.macosx-10.9-x86_64-3.8/chandra_time/XTime.o -Wno-switch-enum -Wno-switch -Wno-switch-default -Wno-deprecated -Wno-parentheses -stdlib=libc++
chandra_time/XTime.cc:782:18: warning: field 'empty' will be initialized after field 'tr' [-Wreorder-ctor]
  : numXTRs (1), empty (1), tr (0) {
                 ^
1 warning generated.
g++ -bundle -undefined dynamic_lookup -L/Users/aldcroft/miniconda3/envs/ska3-dev/lib -arch x86_64 -L/Users/aldcroft/miniconda3/envs/ska3-dev/lib -arch x86_64 -arch x86_64 build/temp.macosx-10.9-x86_64-3.8/chandra_time/_axTime3.o build/temp.macosx-10.9-x86_64-3.8/chandra_time/axTime3.o build/temp.macosx-10.9-x86_64-3.8/chandra_time/XTime.o -o build/lib.macosx-10.9-x86_64-3.8/chandra_time/_axTime3.cpython-38-darwin.so
running install_lib
copying build/lib.macosx-10.9-x86_64-3.8/chandra_time/_axTime3.cpython-38-darwin.so -> /Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages/chandra_time
running install_egg_info
running egg_info
writing chandra_time.egg-info/PKG-INFO
writing dependency_links to chandra_time.egg-info/dependency_links.txt
writing top-level names to chandra_time.egg-info/top_level.txt
reading manifest template 'MANIFEST.in'
warning: no files found matching 'Chandra/Time/*.h'
warning: no files found matching 'Chandra/Time/*.pyx'
adding license file 'LICENSE.rst'
writing manifest file 'chandra_time.egg-info/SOURCES.txt'
removing '/Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages/chandra_time-4.0.2.dev7+g5a88974.d20221222-py3.8.egg-info' (and everything under it)
Copying chandra_time.egg-info to /Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages/chandra_time-4.0.2.dev7+g5a88974.d20221222-py3.8.egg-info
running install_scripts
writing list of installed files to 'record.txt'

Install manifest (sans pycache files)

lib/python3.8/site-packages/chandra_time/Time.py
lib/python3.8/site-packages/chandra_time/__init__.py
lib/python3.8/site-packages/chandra_time/axTime3.py
lib/python3.8/site-packages/chandra_time/tests/test_Time.py
lib/python3.8/site-packages/chandra_time/tests/__init__.py
lib/python3.8/site-packages/Chandra/Time/Time.py
lib/python3.8/site-packages/Chandra/Time/__init__.py
lib/python3.8/site-packages/Chandra/Time/axTime3.py
lib/python3.8/site-packages/Chandra/Time/tests/test_Time.py
lib/python3.8/site-packages/Chandra/Time/tests/__init__.py
lib/python3.8/site-packages/chandra_time/_axTime3.cpython-38-darwin.so
lib/python3.8/site-packages/chandra_time-4.0.2.dev8+gdd9107e-py3.8.egg-info/PKG-INFO
lib/python3.8/site-packages/chandra_time-4.0.2.dev8+gdd9107e-py3.8.egg-info/not-zip-safe
lib/python3.8/site-packages/chandra_time-4.0.2.dev8+gdd9107e-py3.8.egg-info/SOURCES.txt
lib/python3.8/site-packages/chandra_time-4.0.2.dev8+gdd9107e-py3.8.egg-info/top_level.txt
lib/python3.8/site-packages/chandra_time-4.0.2.dev8+gdd9107e-py3.8.egg-info/dependency_links.txt

Test

(ska3-dev) ➜  Chandra.Time git:(de-namespace) cd docs

(ska3-dev) ➜  docs git:(de-namespace) ipython
Python 3.8.12 (default, Oct 12 2021, 06:23:56) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.29.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import Chandra.Time

In [2]: Chandra.Time.__version__
Out[2]: '4.0.2.dev8+gdd9107e'

In [3]: Chandra.Time.__file__
Out[3]: '/Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages/Chandra/Time/__init__.py'

In [4]: Chandra.Time.test()
============================================================================ test session starts ============================================================================
platform darwin -- Python 3.8.12, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages
plugins: remotedata-0.3.3, doctestplus-0.11.2, arraydiff-0.3, anyio-2.2.0, mock-3.6.1, filter-subpackage-0.1.1, openfiles-0.5.0, astropy-header-0.1.2, cov-3.0.0
collected 44 items                                                                                                                                                          

Chandra/Time/tests/test_Time.py ............................................                                                                                          [100%]

============================================================================ 44 passed in 1.79s =============================================================================
Out[4]: False

In [5]: import chandra_time

In [6]: chandra_time.__version__
Out[6]: '4.0.2.dev8+gdd9107e'

In [7]: chandra_time.__file__
Out[7]: '/Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages/chandra_time/__init__.py'

In [8]: chandra_time.test()
============================================================================ test session starts ============================================================================
platform darwin -- Python 3.8.12, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /Users/aldcroft/miniconda3/envs/ska3-dev/lib/python3.8/site-packages
plugins: remotedata-0.3.3, doctestplus-0.11.2, arraydiff-0.3, anyio-2.2.0, mock-3.6.1, filter-subpackage-0.1.1, openfiles-0.5.0, astropy-header-0.1.2, cov-3.0.0
collected 44 items                                                                                                                                                          

chandra_time/tests/test_Time.py ............................................                                                                                          [100%]

============================================================================ 44 passed in 1.15s =============================================================================
Out[8]: False

Build docs

cd docs
make clean
make html

No errors/warnings and the output HTML looked fine.

jeanconn commented 1 year ago

I also confirmed that python setup.py build_ext --inplace still worked for the local build (though I don't know if that is supported or not).

taldcroft commented 1 year ago

I think this is really ready now. All the testing has been redone with the penultimate version, prior to the last commit that just fixed the module docstring.

I had not noticed commits by @jeanconn and got a conflict. Since there were two commits that seemed to do the same thing and one was a merge commit, I just force pushed my new version over those.

jeanconn commented 1 year ago

If that was just https://github.com/sot/chandra_time/pull/55 seems fine.

taldcroft commented 1 year ago

Right, yes it was #55... I have a gut reaction against any merge commits in a PR and didn't stop to wonder where that came from really.

jeanconn commented 1 year ago

OK, so we should probably talk at some point about best practices then? Because I thought doing a PR against yours was the most "polite" way to move things forward without creating conflicts (and let you do the merge as control over content), but that didn't work.

taldcroft commented 1 year ago

A PR on the PR is OK. I would never have even noticed if I had synced the branch prior to making a new commit locally.

In astropy development seeing a merge in the commit history almost always means there is a problem but I got impatient here and didn't stop to think.

taldcroft commented 1 year ago

Still awaiting approval or other comments.