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

Methane Data Collection #36

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

avgagliardo
Copy link
Contributor

@avgagliardo avgagliardo commented Nov 6, 2024

Methane Data Collection

Linting

There is no code included, though the code I used does lint and can be included.

Contents

This PR contains:

  • A set of methane data from the same sites used in the co2 data collection named "methane_data.json"
    • Each record contains fields for the datetime and amount of methane measured
  • An identical set of data, without the extraneous fields removed, named "full_methane_data.json"
  • The original site data .txt file scraped for each site inside of the "raw_site_data" directory

Labels and Milestones

Labels and Milestone have been properly set.

Reviewer

Open to all reviewers, maybe @kylemasc917 since he had a parallel task or someone that is directly downstream would be the most useful in the event they'd like to request changes.

It doesn't take much to reshape the data, so if there's something that I missed or that I can do to make it more convenient just let me know in the review.

Thanks!

@avgagliardo avgagliardo added the Data Anything related to data label Nov 6, 2024
@avgagliardo avgagliardo added this to the Methane Data Collection milestone Nov 6, 2024
@avgagliardo avgagliardo self-assigned this Nov 6, 2024
@haiderabbas007
Copy link
Contributor

Maybe having a LICENSE/NOTES/README would make it better?

@kylemasc917 kylemasc917 self-requested a review November 6, 2024 16:26
@kylemasc917
Copy link
Contributor

Yeah ill review it.

@kylemasc917
Copy link
Contributor

I believe a NOTES txt is required that contains the license and citation for the data other than that It looks good.

@laserlab
Copy link
Contributor

laserlab commented Nov 6, 2024

I believe a NOTES txt is required that contains the license and citation for the data other than that It looks good.

@avgagliardo The easiest and most convenient would be to just use the notes file already existing in the mount Lua folder since it is all the same data source. So I would suggest to @kylemasc917 to switch to approve this PR and then @avgagliardo makes a new PR with moving the NOTES file in the root folder

Copy link
Contributor

@kylemasc917 kylemasc917 left a comment

Choose a reason for hiding this comment

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

Everything looks good Approved.

@avgagliardo
Copy link
Contributor Author

You guys are right, I completely forgot about the README/LICENSE.MD file. I'll take care of that after classes today.

@laserlab
Copy link
Contributor

laserlab commented Nov 6, 2024

You guys are right, I completely forgot about the README/LICENSE.MD file. I'll take care of that after classes today.

DO it in a separate PR though so this can get merged asap @SchrodingersStruggle

@jkblc jkblc self-requested a review November 6, 2024 20:46
Copy link

@jkblc jkblc left a comment

Choose a reason for hiding this comment

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

Looks good. Please merge so I can use the data for testing

@SchrodingersStruggle SchrodingersStruggle merged commit 9f96bbf into ubsuny:main Nov 6, 2024
0 of 2 checks passed
@SchrodingersStruggle
Copy link
Collaborator

Merged it to keep work flowing, @avgagliardo make a separate pull request to add necessary files whenever you get the chance.

@avgagliardo
Copy link
Contributor Author

Sounds good, thank you for the merge everybody. I'll get the license up in a new PR in a few mins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data Anything related to data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants