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

Add complete session start time to FicTrac #598

Merged
merged 9 commits into from
Oct 17, 2023

Conversation

h-mayorquin
Copy link
Collaborator

When working on the Clandining conversion I realized that the first line of the data header has non-valid values save the timestamp. The timestamp is the unix time epoch in milliseconds which can be used to extract a more complete session_start_time as previously we were extracting only the date.

I did some other improvements to the code like adding starting_time when they are not zero which were not done before.

@h-mayorquin
Copy link
Collaborator Author

Oh, I thought that ZoneInfo was introduced in python 3.8 but turns out is not available until python 3.9 : (

Related:
#599

@@ -37,6 +37,10 @@ class TestFicTracDataInterface(DataInterfaceTestMixin, unittest.TestCase):

save_directory = OUTPUT_PATH

def check_extracted_metadata(self, metadata: dict):
Copy link
Member

Choose a reason for hiding this comment

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

Actually, before merge, can we get a test for the new starting time functionality? Or is the example data irregular?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is irregular in the example : (

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I plan to add the timing methods soon, that will be tested there.

Copy link
Member

@CodyCBakerPhD CodyCBakerPhD Oct 17, 2023

Choose a reason for hiding this comment

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

It would be possible to do this once it becomes a Temporally aligned interface; the ability to override those fetched timestamps, you could override it to something regular in a test

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #598 (32df08a) into main (2b568bc) will decrease coverage by 0.11%.
The diff coverage is 59.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #598      +/-   ##
==========================================
- Coverage   91.09%   90.98%   -0.11%     
==========================================
  Files         103      103              
  Lines        5356     5369      +13     
==========================================
+ Hits         4879     4885       +6     
- Misses        477      484       +7     
Flag Coverage Δ
unittests 90.98% <59.25%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...nterfaces/behavior/fictrac/fictracdatainterface.py 70.58% <59.25%> (-1.83%) ⬇️

@CodyCBakerPhD CodyCBakerPhD merged commit 5a7b952 into main Oct 17, 2023
33 of 34 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the add_time_functionality_to_fictrac branch October 17, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants