Enable user to import weather file as .dat #3713
Conversation
WalkthroughThe pull request introduces several significant changes across multiple files. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Config
participant WeatherPathParameter
participant WeatherHelper
participant GenerateWeatherDataSources
User->>Config: Specify weather file
Config->>WeatherPathParameter: Check file type
WeatherPathParameter->>WeatherHelper: If .dat, call parse_dat_weather_file
WeatherHelper->>WeatherHelper: Convert .dat to .epw
WeatherPathParameter->>Config: Validate file paths
User->>GenerateWeatherDataSources: Request weather data sources
GenerateWeatherDataSources->>GenerateWeatherDataSources: Fetch and process Excel files
GenerateWeatherDataSources->>User: Provide GeoJSON output
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (2)
cea/datamanagement/weather_helper/weather_helper.py (1)
77-101: Add documentation for the .dat file formatThe function lacks documentation about the expected structure of the DWD .dat file format.
Please enhance the docstring to include:
- Expected column structure
- Units of measurement
- Sample data format
- Link to DWD documentation
🧰 Tools
🪛 Ruff (0.8.0)
101-101: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
cea/default.config (1)
135-135: LGTM! Consider enhancing the help text.The help text accurately reflects the new capability to handle .dat files. The change aligns well with the PR objective of enabling DWD weather data import.
Consider adding a note about DWD data specifically, since that's the primary use case:
-weather.help = Either a full path to a weather file (.epw or .dat) or the name of one of the weather files contained within the CEA. Leave blank to fetch from third-party sources. +weather.help = Either a full path to a weather file (.epw or .dat - supports German Weather Agency DWD format) or the name of one of the weather files contained within the CEA. Leave blank to fetch from third-party sources.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
cea/config.py(1 hunks)cea/datamanagement/weather_helper/generate_weather_data_sources.py(0 hunks)cea/datamanagement/weather_helper/weather_helper.py(2 hunks)cea/default.config(1 hunks)
💤 Files with no reviewable changes (1)
- cea/datamanagement/weather_helper/generate_weather_data_sources.py
🧰 Additional context used
🪛 Ruff (0.8.0)
cea/datamanagement/weather_helper/weather_helper.py
101-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (1)
cea/config.py (1)
578-578: LGTM: Error message update is clear and accurate
The error message has been appropriately updated to include .dat in the list of supported formats, which aligns with the new functionality.
| if weather.endswith('.dat'): | ||
| print(f"Detected .dat file: {weather}") | ||
| # convert .dat in .epw | ||
| dat_weather_file = weather | ||
| epw_weather_file = locator.get_weather_file() | ||
| parse_dat_weather_file(dat_weather_file, epw_weather_file) | ||
| elif config.weather_helper.weather == 'climate.onebuilding.org': |
There was a problem hiding this comment.
Add file validation and error handling
The .dat file handling needs additional validation and cleanup.
if weather.endswith('.dat'):
+ if not os.path.exists(weather):
+ raise ValueError(f"Weather file not found: {weather}")
print(f"Detected .dat file: {weather}")
- # convert .dat in .epw
dat_weather_file = weather
epw_weather_file = locator.get_weather_file()
+ try:
parse_dat_weather_file(dat_weather_file, epw_weather_file)
+ except Exception as e:
+ # Clean up any partially written EPW file
+ if os.path.exists(epw_weather_file):
+ os.remove(epw_weather_file)
+ raise📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if weather.endswith('.dat'): | |
| print(f"Detected .dat file: {weather}") | |
| # convert .dat in .epw | |
| dat_weather_file = weather | |
| epw_weather_file = locator.get_weather_file() | |
| parse_dat_weather_file(dat_weather_file, epw_weather_file) | |
| elif config.weather_helper.weather == 'climate.onebuilding.org': | |
| if weather.endswith('.dat'): | |
| if not os.path.exists(weather): | |
| raise ValueError(f"Weather file not found: {weather}") | |
| print(f"Detected .dat file: {weather}") | |
| dat_weather_file = weather | |
| epw_weather_file = locator.get_weather_file() | |
| try: | |
| parse_dat_weather_file(dat_weather_file, epw_weather_file) | |
| except Exception as e: | |
| # Clean up any partially written EPW file | |
| if os.path.exists(epw_weather_file): | |
| os.remove(epw_weather_file) | |
| raise | |
| elif config.weather_helper.weather == 'climate.onebuilding.org': |
| elif os.path.exists(value) and value.endswith('.dat'): | ||
| # if dat convert to epw | ||
| dat_weather_file = value | ||
| epw_weather_file = os.path.join(os.path.dirname(value), os.path.basename(value).replace('.dat', '.epw')) | ||
| from cea.datamanagement.weather_helper import parse_dat_weather_file | ||
| parse_dat_weather_file(dat_weather_file, epw_weather_file) | ||
| weather_path = epw_weather_file |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
@Mar-Ges we should only return the path of the file when parsing the weather path parameter in the config. The conversion would be done inside the weather-helper instead.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
@Mar-Ges thank you so much for this PR. Would you be able to provide us with an example of the .dat file or where to get it, so that we can test it on our end as well. |
Added missing lines! (Sorry, I had a problem with my version controll).
Added Error description as suggested
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cea/datamanagement/weather_helper/weather_helper.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
cea/datamanagement/weather_helper/weather_helper.py
112-112: SyntaxError: Expected except or finally after try block
112-112: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
112-112: SyntaxError: Simple statements must be separated by newlines or semicolons
112-112: SyntaxError: Expected a statement
112-113: SyntaxError: Expected an expression
113-113: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
113-113: SyntaxError: Simple statements must be separated by newlines or semicolons
113-113: SyntaxError: Simple statements must be separated by newlines or semicolons
113-114: SyntaxError: Expected 'import', found newline
114-114: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
114-114: SyntaxError: Simple statements must be separated by newlines or semicolons
114-114: SyntaxError: Expected a statement
114-115: SyntaxError: Expected an expression
115-115: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
115-115: SyntaxError: Simple statements must be separated by newlines or semicolons
115-115: SyntaxError: Simple statements must be separated by newlines or semicolons
115-116: SyntaxError: Expected 'import', found newline
116-116: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
116-116: SyntaxError: Simple statements must be separated by newlines or semicolons
116-116: SyntaxError: Expected a statement
116-117: SyntaxError: Expected an expression
117-117: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
117-117: SyntaxError: Simple statements must be separated by newlines or semicolons
117-117: SyntaxError: Simple statements must be separated by newlines or semicolons
🔇 Additional comments (3)
cea/datamanagement/weather_helper/weather_helper.py (3)
112-117: LGTM: Error handling improvements
The error handling has been improved as suggested in the past review, properly preserving the exception context.
🧰 Tools
🪛 Ruff (0.8.0)
112-112: SyntaxError: Expected except or finally after try block
112-112: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
112-112: SyntaxError: Simple statements must be separated by newlines or semicolons
112-112: SyntaxError: Expected a statement
112-113: SyntaxError: Expected an expression
113-113: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
113-113: SyntaxError: Simple statements must be separated by newlines or semicolons
113-113: SyntaxError: Simple statements must be separated by newlines or semicolons
113-114: SyntaxError: Expected 'import', found newline
114-114: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
114-114: SyntaxError: Simple statements must be separated by newlines or semicolons
114-114: SyntaxError: Expected a statement
114-115: SyntaxError: Expected an expression
115-115: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
115-115: SyntaxError: Simple statements must be separated by newlines or semicolons
115-115: SyntaxError: Simple statements must be separated by newlines or semicolons
115-116: SyntaxError: Expected 'import', found newline
116-116: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
116-116: SyntaxError: Simple statements must be separated by newlines or semicolons
116-116: SyntaxError: Expected a statement
116-117: SyntaxError: Expected an expression
117-117: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
117-117: SyntaxError: Simple statements must be separated by newlines or semicolons
117-117: SyntaxError: Simple statements must be separated by newlines or semicolons
129-135:
Add file validation and error handling
The .dat file handling needs additional validation and cleanup.
As suggested in the past review, add file validation and cleanup:
if weather.endswith('.dat'):
+ if not os.path.exists(weather):
+ raise ValueError(f"Weather file not found: {weather}")
print(f"Detected .dat file: {weather}")
dat_weather_file = weather
epw_weather_file = locator.get_weather_file()
+ try:
parse_dat_weather_file(dat_weather_file, epw_weather_file)
+ except Exception as e:
+ if os.path.exists(epw_weather_file):
+ os.remove(epw_weather_file)
+ raise88-90:
Add validation for required columns
The code assumes a specific structure of the .dat file without validation. This could lead to runtime errors if the file format differs.
Add validation for required columns:
weather_data = pd.read_csv(source_dat_file, sep="\s+", skiprows=6, header=None)
+ required_columns = ['RW', 'HW', 'MM', 'DD', 'HH', 't', 'p', 'WR', 'WG', 'N', 'RF', 'B', 'D', 'A']
weather_data.columns = ['RW', 'HW', 'MM', 'DD', 'HH', 't', 'p', 'WR', 'WG', 'N', 'x', 'RF', 'B', 'D', 'A', 'E',
'IL']
+ missing_columns = set(required_columns) - set(weather_data.columns)
+ if missing_columns:
+ raise ValueError(f"Missing required columns in .dat file: {', '.join(missing_columns)}")Likely invalid or redundant comment.
|
|
||
| # from dat to epw | ||
| with open(output_epw_file, 'w') as epw_file: | ||
| epw_file.write("LOCATION,Weimar,-,DEU,SRC-TMYx,105550,50.983,11.317,1,268\n") |
There was a problem hiding this comment.
Replace hardcoded location with configurable parameters
The location information is hardcoded for Weimar, Germany. This limits the function's reusability for other locations.
Consider adding location parameters to the function signature:
-def parse_dat_weather_file(source_dat_file, output_epw_file):
+def parse_dat_weather_file(source_dat_file, output_epw_file, location_name="Weimar", country="DEU",
+ latitude=50.983, longitude=11.317, elevation=268):Then update the location line:
- epw_file.write("LOCATION,Weimar,-,DEU,SRC-TMYx,105550,50.983,11.317,1,268\n")
+ epw_file.write(f"LOCATION,{location_name},-,{country},SRC-TMYx,105550,{latitude},{longitude},1,{elevation}\n")Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
You would need to remove these hardcoded values and use the correct values that are included in the comment header of the .dat file (the first 5 lines of the .dat file if I am not wrong). This is important for DAYSIM to know where to position the scene during raytracing.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Hi @reyery , You can find the german weather database here (for 1 x 1 km squares): Attached an example for Weimar |
| elif os.path.exists(value) and value.endswith('.dat'): | ||
| # if dat convert to epw | ||
| dat_weather_file = value | ||
| epw_weather_file = os.path.join(os.path.dirname(value), os.path.basename(value).replace('.dat', '.epw')) | ||
| from cea.datamanagement.weather_helper import parse_dat_weather_file | ||
| parse_dat_weather_file(dat_weather_file, epw_weather_file) | ||
| weather_path = epw_weather_file |
There was a problem hiding this comment.
@Mar-Ges we should only return the path of the file when parsing the weather path parameter in the config. The conversion would be done inside the weather-helper instead.
| weather_data = pd.read_csv(source_dat_file, sep="\s+", skiprows=6, header=None) | ||
| weather_data.columns = ['RW', 'HW', 'MM', 'DD', 'HH', 't', 'p', 'WR', 'WG', 'N', 'x', 'RF', 'B', 'D', 'A', 'E', | ||
| 'IL'] |
There was a problem hiding this comment.
Based on the sample files that you provided, you could do the following instead.
| weather_data = pd.read_csv(source_dat_file, sep="\s+", skiprows=6, header=None) | |
| weather_data.columns = ['RW', 'HW', 'MM', 'DD', 'HH', 't', 'p', 'WR', 'WG', 'N', 'x', 'RF', 'B', 'D', 'A', 'E', | |
| 'IL'] | |
| weather_data = pd.read_csv(source_dat_file, sep="\s+", skiprows=32, header=0).drop(0) |
This would skip all the comment header lines and read the provided column names from the file.
We could use the list of column names to compare with the dataframe to make sure that whatever that is read is correct.
e.g.
dat_columns = ['RW', 'HW', 'MM', 'DD', 'HH', 't', 'p', 'WR', 'WG', 'N', 'x', 'RF', 'B', 'D', 'A', 'E', 'IL']
if not set(dat_columns) == set(weather_data.columns):
raise ValueError("Invalid .dat file")| def parse_dat_weather_file(source_dat_file, output_epw_file): | ||
| """ | ||
| convert a DWD .dat-file to a epw-file | ||
|
|
There was a problem hiding this comment.
Would you be able to add the header definitions of the .dat file in english in the function's docstring as well. I think this would help others understand the provided columns when debugging this in the future.
Reihenfolge der Parameter:
RW Rechtswert [m] {3670500;3671500..4389500}
HW Hochwert [m] {2242500;2243500..3179500}
MM Monat {1..12}
DD Tag {1..28,30,31}
HH Stunde (MEZ) {1..24}
t Lufttemperatur in 2m Hoehe ueber Grund [GradC]
p Luftdruck in Standorthoehe [hPa]
WR Windrichtung in 10 m Hoehe ueber Grund [Grad] {0..360;999}
WG Windgeschwindigkeit in 10 m Hoehe ueber Grund [m/s]
N Bedeckungsgrad [Achtel] {0..8;9}
x Wasserdampfgehalt, Mischungsverhaeltnis [g/kg]
RF Relative Feuchte in 2 m Hoehe ueber Grund [Prozent] {1..100}
B Direkte Sonnenbestrahlungsstaerke (horiz. Ebene) [W/m^2] abwaerts gerichtet: positiv
D Diffuse Sonnenbetrahlungsstaerke (horiz. Ebene) [W/m^2] abwaerts gerichtet: positiv
A Bestrahlungsstaerke d. atm. Waermestrahlung (horiz. Ebene) [W/m^2] abwaerts gerichtet: positiv
E Bestrahlungsstaerke d. terr. Waermestrahlung [W/m^2] aufwaerts gerichtet: negativ
IL Qualitaetsbit bezueglich der Auswahlkriterien {0;1;2;3;4}
|
|
||
| # from dat to epw | ||
| with open(output_epw_file, 'w') as epw_file: | ||
| epw_file.write("LOCATION,Weimar,-,DEU,SRC-TMYx,105550,50.983,11.317,1,268\n") |
There was a problem hiding this comment.
You would need to remove these hardcoded values and use the correct values that are included in the comment header of the .dat file (the first 5 lines of the .dat file if I am not wrong). This is important for DAYSIM to know where to position the scene during raytracing.
| + except pd.errors.EmptyDataError as e: | ||
| + raise ValueError("The .dat file is empty or has incorrect format") from e | ||
| + except pd.errors.ParserError as e: | ||
| + raise ValueError("Failed to parse the .dat file - invalid format") from e | ||
| + except Exception as e: | ||
| + raise ValueError(f"Unexpected error while parsing .dat file: {str(e)}") from e |
There was a problem hiding this comment.
There are additional characters at the start of these lines that have to be removed.
Hi all,
This update is especially for anyone using data from the German Weather Agency (DWD), which provides weather files in this format.
Summary by CodeRabbit
Release Notes
New Features
.datweather files, which can now be converted to.epwformat.Bug Fixes
Documentation