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

Make pr_get_daynight generic #116

Open
jaseeverett opened this issue Apr 18, 2022 · 4 comments
Open

Make pr_get_daynight generic #116

jaseeverett opened this issue Apr 18, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request high priority Issue that should be dealt with first

Comments

@jaseeverett
Copy link
Contributor

At the moment pr_get_daynight reloads the CPR data and doesn't work with NRS. I need to make this function more generic so that it can be used with any data frame that has a time.

@jaseeverett jaseeverett self-assigned this Apr 18, 2022
@jaseeverett jaseeverett added the medium priority Issue that is important but not critical label Apr 18, 2022
@jaseeverett jaseeverett added high priority Issue that should be dealt with first and removed medium priority Issue that is important but not critical labels Jun 22, 2022
@jaseeverett
Copy link
Contributor Author

I have started work on this. At this stage I had created a new pr_add_daynight function and kept the old one till we can fully debug it and work out all the implications of replacing it.

@jaseeverett jaseeverett added the enhancement New feature or request label Jun 23, 2022
@jaseeverett
Copy link
Contributor Author

Due to the current "complete" command in the indices (and elsewhere) this leaves a lot of NAs for lat/lon which causes trouble with tz's. Need to discuss with @clairedavies how to deal with complete.

@clairedavies
Copy link
Contributor

I disagree - we should only use the CPR data unless we have another big dataset that has day and night data. The NRS doesn't. We get into issues of non-comparable abundance data if we use more than one dataset and this isn't worth it when a dataset only has daytime data.
We also got rid of complete, so is your second comment still valid?

@jaseeverett
Copy link
Contributor Author

I think you misunderstand and also I didn't explain very well! You are right. NRS is not useful. That was a bad example.

At the moment pr_get_daynight loads the CPR data from the AODN and calculates day night. This is silly if the user already has the CPR data in their workspace. It was mainly written for the BOO diurnal stuff.

For a user of planktonr (not BOO) this function would be much more useful if you load the CPR data using pr_get_CPRData() and pass it to pr_get_daynight. That way we could make it also work for any user's dataset that has a time and lat/lon column. Additionally, pr_get_daynight is hardwired for copepods (Z) or species (P) but users may want it for other groups as well. Again, ingesting a df into the function would make it more flexible.

I started working on this here:

# Add day/night marker to dataframe

but haven't finished it yet. Will come back to it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high priority Issue that should be dealt with first
Projects
None yet
Development

No branches or pull requests

2 participants