Skip to content

Change date format, column names and order for jasmine daily summaries#285

Open
MMel099 wants to merge 16 commits intoonnela-lab:developfrom
MMel099:change_jasmine_date_format
Open

Change date format, column names and order for jasmine daily summaries#285
MMel099 wants to merge 16 commits intoonnela-lab:developfrom
MMel099:change_jasmine_date_format

Conversation

@MMel099
Copy link

@MMel099 MMel099 commented Mar 26, 2025

Previously, the Jasmine daily summaries have three separate columns for day, month and year. This pull request changes only the daily summary to have the format MM/DD/YYYY (EDIT: SINCE CHANGED TO YYYY-MM-DD)

@MMel099 MMel099 marked this pull request as draft March 26, 2025 17:23
@MMel099
Copy link
Author

MMel099 commented Mar 27, 2025

Holding on this for now.

@MMel099
Copy link
Author

MMel099 commented May 1, 2025

Commit 8600d9a (Adjust Jasmine output column names and order) changes the column names and order to match the Jasmine output generated by the dashboard server's Forest tasks.

IMPORTANT: If parameters.split_day_night is set to TRUE, a downstream collision will occur with the split_day_night_cols function due to the new column names. A future commit will be added to address this.

@MMel099
Copy link
Author

MMel099 commented May 15, 2025

An if statement check was added based on parameters.split_day_night. If it is set to false, the names and order are updated. Otherwise, the old format is maintained.

@MMel099 MMel099 marked this pull request as ready for review May 15, 2025 16:35
@MMel099 MMel099 marked this pull request as draft May 15, 2025 18:32
@MMel099 MMel099 marked this pull request as ready for review May 15, 2025 18:32
@biblicabeebli biblicabeebli requested a review from hackdna May 15, 2025 18:36
@biblicabeebli
Copy link
Member

(for some reason @MMel099 could not add @hackdna as a reviewer.)

@hackdna
Copy link
Member

hackdna commented May 15, 2025

@GeorgeEfstathiadis could you have a look? I think you were the original author.

Copy link
Member

@hackdna hackdna left a comment

Choose a reason for hiding this comment

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

@MMel099 Thank you for submitting this PR. Could you merge the latest changes from the develop branch and push the updated version to trigger our test workflows in GitHub Actions?

@hackdna
Copy link
Member

hackdna commented May 15, 2025

@MMel099 I was able to trigger the test workflows but the checks are failing (https://github.com/onnela-lab/forest/actions/runs/15055312103/job/42319759802?pr=285).

Please make sure all the checks are passing (for example, remove the commented out code; here's the Forest development guide for reference: https://forest.beiwe.org/en/latest/development.html) and then we will continue the review process. Thanks!

@biblicabeebli
Copy link
Member

I need a confirmation that these changes retain the current behavior as the default behavior, without changing output ordering and field names when run on a Beiwe server. (it can change, I just need to know.)

8600d9a8d2695db0c7b288f45911c40e7e22a4e6

@MMel099
Copy link
Author

MMel099 commented May 19, 2025

Hi @hackdna and @biblicabeebli,

Thanks for helping move this along!

Ilya, I will look at these errors and push out a commit addressing them.

Eli, these changes modify the column headers and order of the default forest jasmine output to mimic what is produced on the server. For example, the current Forest package produces a Jasmine field called max_dist_home. The jasmine summaries generated by the server call this field Distance From Home.

I guess this is a good time to ask how the server uses the Forest package, and at what point do the headers/order get modified?

Thanks!

@biblicabeebli
Copy link
Member

(aw crap I totally missed that response.)

current

  • code: we have a dictionary that maps the incoming column names to internal database column names, we can change column names, it just needs to be very clearly documented somewhere what these should be. This is complex to resolve because we have at least 3 consumers of this data and people are super picky about names.
  • https://github.com/onnela-lab/beiwe-discussions/issues/290 is an internal issue to review, currently @MMel099 @biblicabeebli @hydawo
  • @hydawo has a reminder to create a new issue for a review of fields, we are waiting on that.

@hackdna
Copy link
Member

hackdna commented Jun 2, 2025

Also, the date should be in YYYY-MM-DD format which is an international standard.
https://en.wikipedia.org/wiki/ISO_8601

@MMel099
Copy link
Author

MMel099 commented Jun 3, 2025

At the outset of this pull request, the goal was just to change the date and time conventions for the Jasmine summaries which had separate rows for date, hour, minute, etc. We later added the goal of fixing the discrepancy between the naming conventions of the local Forest script and those used to generate the dashboard summaries.

Just realized that I should also check the other trees (Oak, Willow and possibly even Sycamore) for this discrepancy before rolling anything out.

  • Check/Fix naming conventions in other trees

Just out of curiousity about the next steps - who are the 3 consumers of this data that we would have to consult before making changes to the naming conventions? If we do proceed with these, what would be the best way of communicating changes - something like a 2 column csv with the old column names and the new ones?

Lastly, I want to note that Eli advised me to use the YYYY-MM-DD format which I implemented (I should go back and edit my previous comments in this thread)

@MMel099 MMel099 changed the title Change date format for jasmine daily summaries Change date format, column names and order for jasmine daily summaries Jun 3, 2025
@biblicabeebli
Copy link
Member

Consumers - I'm speaking very generally here, a consumer is anyone who uses the platform that is a specific target audience for which there is a different kind of communication to be thought about. This time I came up with more. (It might seem ultra pedantic, but this is precisely the "the hardest problem in computer science is naming things correctly" Thing, if you've ever heard that.):

  • you, you are one of the consumers, you are a developer of data science stuff with special knowledge and expertise
  • external developers/data scientists connected to the forest platform, but without expertise.
  • I, the developer of the backend, am a consumer at a pedantic code level
  • other developers of the backend who have even less.
  • customers of the service center, with a highly varied background and level of expertise.
  • public users of the platform that are not connected to the service center, using another person's instance that can't reach out to an expert for a clarification.

@hydawo
Copy link
Collaborator

hydawo commented Jun 23, 2025

@MMel099 could you work on a new mapping dictionary for Eli? I think a simple table showing those fields (from all trees) whose headers are changing. @biblicabeebli I believe this is what we discussed recently and more or less what you need?

@hackdna let's move forward with an initial release of Forest with the 3.12 upgrade. We can push another release once these new changes that @MMel099 is making (in conjuction with @biblicabeebli ) are ironed out.

Please let me know if I'm misunderstanding anything. Thanks!

@MMel099
Copy link
Author

MMel099 commented Jun 24, 2025

@hydawo I will push on this issue over the weekend and attach that column mapping here

@biblicabeebli
Copy link
Member

I need a commit or forest release version to do the server side update.

@MMel099
Copy link
Author

MMel099 commented Jun 29, 2025

@hydawo @biblicabeebli @hackdna

I have revised the branch to ensure that all github tests pass before merging. I'm also attaching a csv mapping with old and new column names with a few extra comments. Let me know if I should make any further changes or provide any other resources. Thank you!

column_mapping.csv

@biblicabeebli
Copy link
Member

@hydawo @hackdna @MMel099 Has this pull request been sitting here ready to merge since end of June, or has it been outmoded?

@hackdna
Copy link
Member

hackdna commented Nov 7, 2025

I was waiting for a go ahead from @biblicabeebli

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.

4 participants