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

Fix sunrise/sunset scatter plots #16

Merged
merged 4 commits into from
Mar 12, 2024
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 12 additions & 22 deletions scripts/plotly_streamlit.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import plotly.express as px
from sklearn.preprocessing import normalize
from suntime import Sun
from datetime import datetime

profile = False
if profile:
Expand Down Expand Up @@ -181,25 +180,21 @@ def time_resample(df, resample_time):
font_size = 15


def sunrise_sunset_scatter(num_days_to_display):
latitude = df2['Lat'][0]
longitude = df2['Lon'][0]
def sunrise_sunset_scatter(date_range):
latitude = df2['Lat'][-1]
Copy link
Author

@cdkl cdkl Mar 8, 2024

Choose a reason for hiding this comment

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

Use the most recent lat/long in the db for finding sunrise/sunset times.

Not strictly necessary but I had a secondary problem that others might have: My initial lat/long configuration (back when I first set up birdnet-pi) was wrong. Why does this matter? Well, the existing code uses the earliest lat/long it can find in the database! I cleaned up my data a bit, but decided to include this change.

This is a simple change that uses the latest lat/long in the database. I figure that's going to be as or more correct pretty much all the time. (Ideally, this would instead do an actual configuration lookup instead of inferring from db detection records.)

Choose a reason for hiding this comment

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

Well-done! That is a good find! There's been quite a lot of people over the years who didn't initially set their lat/lon and did it afterwards. That would indeed result in the problem. I see what you mean about config lookup too. I agree that it would be ideal. Never looked at the code in Species Stats. Might have to familiarise myself with that!

longitude = df2['Lon'][-1]
Copy link
Owner

Choose a reason for hiding this comment

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

You could import

def get_settings(settings_path='/etc/birdnet/birdnet.conf', force_reload=False):

and then:

Suggested change
latitude = df2['Lat'][-1]
longitude = df2['Lon'][-1]
conf = get_settings()
latitude = conf.getfloat('LATITUDE')
longitude = conf.getfloat('LONGITUDE')

If you don't feel like doing that (now), please add a your explanation in a comment.

Also, if your git foo allows it, I would prefer this change and the actual fix in two separate commits.
Apart from these nitpicks, great fix, Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Yes! that's a great idea. This can be a separate change. Let me adjust this merge request to use the old lat/lon method, and I'll prep separate changes for this.


sun = Sun(latitude, longitude)

sunrise_list = []
sunset_list = []
sunrise_week_list = []
sunset_week_list = []
sunrise_text_list = []
sunset_text_list = []
daysback_range = []

now = datetime.now()
current_date = start_date

for past_day in range(num_days_to_display):
d = timedelta(days=num_days_to_display - past_day - 1)

current_date = now - d
for current_date in date_range:
# current_date = datetime.fromisocalendar(2022, week + 1, 5)
# time_zone = datetime.now()
sun_rise = sun.get_local_sunrise_time(current_date)
Expand All @@ -214,17 +209,17 @@ def sunrise_sunset_scatter(num_days_to_display):
sunset_text_list.append(temp_time)
sunrise_list.append(sun_rise_time)
sunset_list.append(sun_dusk_time)
sunrise_week_list.append(past_day)
sunset_week_list.append(past_day)

sunrise_week_list.append(None)
daysback_range.append(current_date.strftime('%d-%m-%Y'))

sunrise_list.append(None)
sunrise_text_list.append(None)
sunrise_list.extend(sunset_list)
sunrise_week_list.extend(sunset_week_list)
sunrise_text_list.extend(sunset_text_list)
daysback_range.append(None)
daysback_range.extend(daysback_range)

return sunrise_week_list, sunrise_list, sunrise_text_list
return daysback_range, sunrise_list, sunrise_text_list


def hms_to_dec(t):
Expand Down Expand Up @@ -464,12 +459,7 @@ def hms_to_str(t):
# text=labels,
texttemplate="%{text}", autocolorscale=False, colorscale=selected_pal
)
num_days_to_display = len(fig_x)
sunrise_week_list, sunrise_list, sunrise_text_list = sunrise_sunset_scatter(num_days_to_display)
daysback_range = fig_x
daysback_range.append(None)
daysback_range.extend(daysback_range)
daysback_range = daysback_range[:-1]
daysback_range, sunrise_list, sunrise_text_list = sunrise_sunset_scatter(day_hour_freq.index.tolist())

sunrise_sunset = go.Scatter(x=daysback_range,
y=sunrise_list,
Expand Down
Loading