-
Notifications
You must be signed in to change notification settings - Fork 19
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
added plot_map #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition! Some comments and suggestions :)
|
||
A map of activities viewed in plan. | ||
|
||
![map](https://github.com/marcusvolz/strava_py/blob/main/plots/map001.png "A map of activities viewed in plan.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove full stop to match the facets alt text.
![map](https://github.com/marcusvolz/strava_py/blob/main/plots/map001.png "A map of activities viewed in plan.") | |
![map](https://github.com/marcusvolz/strava_py/blob/main/plots/map001.png "A map of activities viewed in plan") |
parser.add_argument("lon_min", default=None, help="Minimum longitude for plot_map (values less than this are removed from the data)") | ||
parser.add_argument("lon_max", default=None, help="Maximum longitude for plot_map (values greater than this are removed from the data)") | ||
parser.add_argument("lat_min", default=None, help="Minimum latitude for plot_map (values less than this are removed from the data)") | ||
parser.add_argument("lat_max", default=None, help="Maximum latitude for plot_map (values greater than this are removed from the data)") | ||
parser.add_argument("alpha", default=0.4, help="Line transparency. 0 = Fully transparent, 1 = No transparency") | ||
parser.add_argument("linewidth", default=0.4, help="Line width") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include --
prefixes otherwise it complains about:
usage: strava_py [-h] [-o OUTPUT_FILE] path lon_min lon_max lat_min lat_max alpha linewidth
strava_py: error: the following arguments are required: lon_min, lon_max, lat_min, lat_max, alpha, linewidth
parser.add_argument("lon_min", default=None, help="Minimum longitude for plot_map (values less than this are removed from the data)") | |
parser.add_argument("lon_max", default=None, help="Maximum longitude for plot_map (values greater than this are removed from the data)") | |
parser.add_argument("lat_min", default=None, help="Minimum latitude for plot_map (values less than this are removed from the data)") | |
parser.add_argument("lat_max", default=None, help="Maximum latitude for plot_map (values greater than this are removed from the data)") | |
parser.add_argument("alpha", default=0.4, help="Line transparency. 0 = Fully transparent, 1 = No transparency") | |
parser.add_argument("linewidth", default=0.4, help="Line width") | |
parser.add_argument("--lon_min", default=None, help="Minimum longitude for plot_map (values less than this are removed from the data)") | |
parser.add_argument("--lon_max", default=None, help="Maximum longitude for plot_map (values greater than this are removed from the data)") | |
parser.add_argument("--lat_min", default=None, help="Minimum latitude for plot_map (values less than this are removed from the data)") | |
parser.add_argument("--lat_max", default=None, help="Maximum latitude for plot_map (values greater than this are removed from the data)") | |
parser.add_argument("--alpha", default=0.4, help="Line transparency. 0 = Fully transparent, 1 = No transparency") | |
parser.add_argument("--linewidth", default=0.4, help="Line width") |
from strava_py.plot_map import plot_map | ||
from strava_py.plot_facets import plot_facets | ||
from strava_py.process_data import process_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: alphabetical order helps as number of imports grows:
from strava_py.plot_map import plot_map | |
from strava_py.plot_facets import plot_facets | |
from strava_py.process_data import process_data | |
from strava_py.plot_facets import plot_facets | |
from strava_py.plot_map import plot_map | |
from strava_py.process_data import process_data |
|
||
def plot_map(df, lon_min=None, lon_max= None, lat_min=None, lat_max=None, | ||
alpha=0.3, linewidth=0.3, output_file="map.png"): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plot_facets(df, output_file=args.output_file) | ||
print(f"Saved to {args.output_file}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call plot_map
and also pass the CLI args:
plot_facets(df, output_file=args.output_file) | |
print(f"Saved to {args.output_file}") | |
plot_map( | |
df, | |
lon_min=args.lon_min, | |
lon_max=args.lon_max, | |
lat_min=args.lat_min, | |
lat_max=args.lat_max, | |
alpha=args.alpha, | |
linewidth=args.linewidth, | |
output_file=args.output_file, | |
) | |
print(f"Saved to {args.output_file}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're also re-using args.output_file
and therefore writing over the facets image.
Some options:
- Remove the argument entirely
- Add a second one for the map plot
- Change the argument to be a prefix instead (e.g.
strava
), and we append-facets.png
and-map.png
I think the third, as there may be more visualisations added soon, and it will be useful to be able to have different names for different runs (for example, I might generate output for each year of activities, or summer vs. winter).
import matplotlib.pyplot as plt | ||
|
||
def plot_map(df, lon_min=None, lon_max= None, lat_min=None, lat_max=None, | ||
alpha=0.3, linewidth=0.3, output_file="map.png"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defaults are 0.3
here and 0.4
for the CLI. Should they be the same?
No description provided.