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

Updates to current_time #684

Merged
merged 8 commits into from
Oct 10, 2018
Merged

Updates to current_time #684

merged 8 commits into from
Oct 10, 2018

Conversation

wtgee
Copy link
Member

@wtgee wtgee commented Oct 7, 2018

  • Behavior change Increment by one second after reading value. Helps with test problems due to colliding filenames coming from the camera simulator. Also slightly more "realistic"
  • Docs updates

* Increment by one second after reading value.
* Docs updates
@wtgee wtgee added the Quick PR This PR should be quick to review. label Oct 7, 2018
@wtgee wtgee requested a review from jamessynge October 7, 2018 22:33
@codecov
Copy link

codecov bot commented Oct 7, 2018

Codecov Report

Merging #684 into develop will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #684      +/-   ##
===========================================
+ Coverage    77.86%   77.86%   +<.01%     
===========================================
  Files           61       61              
  Lines         5073     5069       -4     
  Branches       694      695       +1     
===========================================
- Hits          3950     3947       -3     
+ Misses         914      912       -2     
- Partials       209      210       +1
Impacted Files Coverage Δ
pocs/camera/camera.py 89.3% <ø> (-0.12%) ⬇️
pocs/serial_handlers/protocol_arduinosimulator.py 73.46% <0%> (-0.77%) ⬇️
pocs/observatory.py 89.77% <0%> (+0.09%) ⬆️
pocs/utils/images/__init__.py 65.82% <0%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6227d1...e6a9be4. Read the comment docs.

@@ -148,6 +148,10 @@ class PocsShell(Cmd):
"""Setup and initialize a POCS instance."""
simulator = listify(arg[0].split())

if 'POCSTIME' in os.environ:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious what motivated this addition? Not asking for a change.

Copy link
Member Author

Choose a reason for hiding this comment

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

From the PR description: "Helps with test problems due to colliding filenames coming from the camera simulator. Also slightly more 'realistic'"

@@ -184,14 +184,14 @@ def test_standard_headers(observatory):
'latitude': 19.54,
'longitude': -155.58,
'moon_fraction': 0.7880103086091879,
'moon_separation': 156.1607340087774,
'moon_separation': 148.34401,
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes seem unrelated to the description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, not sure what is going on here. Commit message was: "@wtgee Changing moon values for this test. Not sure why needed "

@@ -49,6 +63,8 @@ def current_time(flatten=False, datetime=False, pretty=False):

if pocs_time is not None and pocs_time > '':
_time = Time(os.getenv('POCSTIME'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why repeat the call to getenv, rather than use the pocs_time variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

Too much copy/paste, too little thinking. :)

@wtgee wtgee merged commit c31266a into panoptes:develop Oct 10, 2018
@wtgee wtgee deleted the increment-pocstime branch October 10, 2018 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quick PR This PR should be quick to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants