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

Issue/2 #30

Merged
merged 3 commits into from
Jan 9, 2025
Merged

Issue/2 #30

merged 3 commits into from
Jan 9, 2025

Conversation

alirashidAR
Copy link
Contributor

Pull Request #2

Description

Created functionality to pull data from the pv-live library in the fetch_pvlive_data.py file.

Fixes

Summary of Changes

  • Implemented methods to interact with the pv-live library.
  • Added test cases in tests/data/test_pvlive_data.py to verify the functionality.

How Has This Been Tested?

The following tests were implemented and verified using pytest:

  1. Test for get_latest_data: Ensures the method fetches the latest data with the correct parameters.
  2. Test for get_data_between: Validates the method fetches data for a specified time range.
  3. Test for get_data_at_time: Checks the method fetches data for a specific timestamp.
  • Yes

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@jcamier
Copy link
Collaborator

jcamier commented Jan 8, 2025

@alirashidAR this is great. Thanks for doing this. I recommend we change the print statements to logging.

import logging
logger = logging.getLogger(__name__)
...
logger.error(e) instead of print statements (f"Error: {e}") etc.
or 
logger.exception("Problem getting data...) the traceback is already included in this case

@alirashidAR
Copy link
Contributor Author

Sure , I'll make the changes. Thank you !

@alirashidAR
Copy link
Contributor Author

@jcamier I've made the necessary changes , please take a look.

@jcamier
Copy link
Collaborator

jcamier commented Jan 9, 2025

@alirashidAR Looks good to me.
@peterdudfield do you approve to merge?

@jcamier jcamier merged commit a579f23 into openclimatefix:main Jan 9, 2025
@peterdudfield
Copy link
Contributor

Next step @alirashidAR is to run this code, and collect the data from this? Would you be up for this?

Get data at a specific time
"""
try:
df = self.pvl.at_time(dt, entity_type="pes", entity_id=0, extra_fields="", period=30, dataframe=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry i missed this, but entity_type should be "gsp"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, I'll create a pull request to change the default entity type to gsp

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.

3 participants