Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve time conversion helpers #36

Merged
merged 2 commits into from
Apr 5, 2023
Merged

Conversation

taldcroft
Copy link
Member

Description

This improves the time conversion helper utilities which are available as a CLI via the cxotime script or the print_conversions() method of CxoTime.

  • Factor out the conversions bit into a new get_conversions() method.
  • Add parameters to methods that allow for unit testing.
  • CLI script was put into a more standard place in scripts, now being run through main().
  • Conversion to local time is now done using timezone support in a built-in module available in Python 3.10.
    • Unlike the previous method, this gives the expected local time zone even after django.setup() is run if the TZ environment var is removed.

Interface impacts

None

Testing

Unit tests

  • Mac (new unit tests)

Functional tests

In [1]: import kadi.events

In [2]: kadi.events.eclipses
Out[2]: <EventQuery: Eclipse>

In [3]: import os

In [4]: os.environ["TZ"]
Out[4]: 'America/Chicago'

In [5]: del os.environ["TZ"]

In [6]: from cxotime import CxoTime

In [7]: CxoTime("2022:300:01:00:00.000").get_conversions()
Out[7]: 
{'local': '2022 Wed Oct 26 09:00:00 PM EDT',
 'iso_local': '2022-10-26T21:00:00-04:00',
 'date': '2022:300:01:00:00.000',
 'cxcsec': 783219669.184,
 'decimalyear': 2022.8192922374428,
 'iso': '2022-10-27 01:00:00.000',
 'unix': 1666832400.0}

In [8]: os.environ["TZ"] = "America/Chicago"

In [9]: CxoTime("2022:300:01:00:00.000").get_conversions()
Out[9]:
{'local': '2022 Wed Oct 26 08:00:00 PM CDT',
'iso_local': '2022-10-26T20:00:00-05:00',
'date': '2022:300:01:00:00.000',
'cxcsec': 783219669.184,
'decimalyear': 2022.8192922374428,
'iso': '2022-10-27 01:00:00.000',
'unix': 1666832400.0}

Copy link
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new 'scripts' dir needs to be added to the setup.py 'packages' set.

File-like object to write output (default=sys.stdout). Mostly for testing.
"""
if date is None:
if len(sys.argv) > 2:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No biggie, but I naively tried "cxotime --help" and "cxotime -help" and they weren't helpful as that just throws an exception ValueError: Input values did not match any of the formats where the format keyword is optional...

So I'm not sure if there is some value to supporting some kind of usage help.

Copy link
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me now.

@taldcroft taldcroft force-pushed the better-time-conversions branch from 54d85c8 to 0bd1382 Compare April 5, 2023 18:52
@taldcroft taldcroft merged commit 5000fd4 into master Apr 5, 2023
@taldcroft taldcroft deleted the better-time-conversions branch April 5, 2023 18:53
@javierggt javierggt mentioned this pull request Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants