Feat/geo distance improvements #116
Conversation
…rs/minutes/seconds)
|
👋 @Ericbutler1209 👋 We're delighted to have your pull request! Please take a moment to check our contributing guidelines and ensure you've filled out the PR template for a smooth process. We will review it soon. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds several improvements to enhance user experience and code maintainability across multiple Python utilities. The changes focus on input validation, unit conversion, better formatting, and code modernization.
- Added coordinate validation and distance unit conversion to geographic distance calculator
- Enhanced digital clock with 12/24-hour format toggle and date display
- Modernized age calculator with type hints and improved output formatting
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Geographic Distance/geographic_distance.py | Added helper functions for coordinate validation, distance conversion, and time formatting |
| Digital Clock/main.py | Added format toggle functionality, date display, and keyboard shortcuts |
| Age Calculator/calculate.py | Refactored with type hints, improved function names, and enhanced output display |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def validate_coordinates(coord): | ||
| """Ensure latitude and longitude are within valid ranges.""" | ||
| lat, lon = coord | ||
| if not (-90 <= lat <= 90): | ||
| raise ValueError(f"Latitude {lat} out of range (-90..90)") | ||
| if not (-180 <= lon <= 180): | ||
| raise ValueError(f"Longitude {lon} out of range (-180..180)") | ||
| return coord | ||
|
|
||
| def km_to_miles(km: float) -> float: | ||
| """Convert kilometers to miles.""" | ||
| return km * 0.621371 | ||
|
|
||
| def format_travel_time(hours: float) -> str: | ||
| """Format fractional hours as 'Hh Mm'.""" | ||
| h = int(hours) | ||
| m = int(round((hours - h) * 60)) | ||
| if m == 60: | ||
| h += 1 | ||
| m = 0 | ||
| return f"{h}h {m}m" |
There was a problem hiding this comment.
The indentation of these function definitions is inconsistent with Python standards. Function definitions should be at the top level (no indentation) or properly indented within a class. These functions appear to have 4 spaces of indentation when they should be at column 0.
Copilot uses AI. Check for mistakes.
| @@ -1,23 +1,50 @@ | |||
| from time import strftime | |||
| from tkinter import Label, Tk | |||
| from tkinter import Label, Tk , Button | |||
There was a problem hiding this comment.
Extra space before the comma in the import statement. Should be 'Tk, Button' instead of 'Tk , Button'.
| from tkinter import Label, Tk , Button | |
| from tkinter import Label, Tk, Button |
Copilot uses AI. Check for mistakes.
| minutes = hours * 60 | ||
| seconds = minutes * 60 |
There was a problem hiding this comment.
The calculations create unnecessary intermediate variables and potential integer overflow. Consider calculating directly: hours = day * 24, minutes = day * 24 * 60, seconds = day * 24 * 60 * 60 to avoid accumulating rounding errors and reduce memory usage.
| minutes = hours * 60 | |
| seconds = minutes * 60 | |
| minutes = day * 24 * 60 | |
| seconds = day * 24 * 60 * 60 |
Copilot uses AI. Check for mistakes.
Added validate_coordinates() helper to cleanly validate latitude/longitude inputs
Added km_to_miles() and updated output to show both kilometers and miles
Added format_travel_time() to display travel time in hours and minutes instead of raw decimals