search

singularity-energy / open-grid-emissions

Tools for producing high-quality hourly generation and emissions data for U.S. electric grids
MIT License
67 stars 4 forks source link

Fix filepaths for s3 #346

Closed grgmiller closed 4 months ago

grgmiller commented 4 months ago

Purpose

When importing OGE into another module on windows, and attempting to load data from s3, the current way filepaths are handled results in a FileNotFound error.

For example, running oge.filepaths.results_folder("2022/plant_data/plant_static_attributes.csv") returns:

's3://open-grid-emissions/open_grid_emissions_data\\results\\2022/plant_data/plant_static_attributes.csv'

Where os.path.join() is concatting parts of the filepath together, it is adding a "\" rather than a "/". When attempting to read from this location, for example using pd.read_csv(oge.filepaths.results_folder("2022/plant_data/plant_static_attributes.csv"), this results in:

FileNotFoundError: open-grid-emissions/open_grid_emissions_data\results\2022/plant_data/plant_static_attributes.csv

Here, the mix of forward and back slashes does not seem to be compatible with reading from s3. If instead I try pd.read_csv(oge.filepaths.results_folder("2022/plant_data/plant_static_attributes.csv").replace("\\","/")), this successfully returns the dataframe

What the code is doing

This PR adds .replace("\\","/") to all of the functions that return folder locations where data is saved to help prevent this issue in any package that imports OGE and reads the data from s3.

Testing

Did a fresh install of the local oge version, and ran the pipeline long enough to check that local data loading still works.

Where to look

Usage Example/Visuals

Review estimate

Code review: < 5 min. If you test on a mac, it may take longer.

Future work

What issues were identified that are not being addressed in this PR but should be addressed in future work?

Checklist

rouille commented 4 months ago

I am worried that by fixing the path for s3 we break the path for Windows' users running locally

grgmiller commented 4 months ago

I am worried that by fixing the path for s3 we break the path for Windows' users running locally

I did test this locally on my windows laptop and this still works to load the data. I've never had an issue using forward slashes for filepaths on windows

Testing Did a fresh install of the local oge version, and ran the pipeline long enough to check that local data loading still works.