Skip to content

Conversation

@b-reyes
Copy link
Contributor

@b-reyes b-reyes commented Nov 21, 2022

This PR addresses issue #19 by changing all data column names to those specified in echopro_final_column_names. Additionally, the following items were completed:

  • All data files within the Google Drive folder 2019_consolidated_files were updated
  • All notebooks were reran and changed to reflect the updated names

Note: This PR is currently a draft because of the comment in issue 19, which suggests changing fraction_hake to fraction_species.

@b-reyes b-reyes added this to the Release 0.1.1-alpha milestone Nov 21, 2022
@b-reyes b-reyes requested a review from emiliom November 21, 2022 23:49
@emiliom
Copy link
Contributor

emiliom commented Nov 22, 2022

Awesome, thanks! I'll review this PR tomorrow.

@b-reyes
Copy link
Contributor Author

b-reyes commented Nov 22, 2022

Restating for full clarity: In #34 we have come to a collective conclusion that referencing Hake is fine for the current state of EchoPro. For this reason, we will continue to use the name fraction_hake.

@b-reyes b-reyes marked this pull request as ready for review November 22, 2022 16:22
Copy link
Contributor

@emiliom emiliom left a comment

Choose a reason for hiding this comment

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

Looks good! I made some small suggestions, just to improve consistency or add clarity. I don't think I ran into any actual problems.

@b-reyes
Copy link
Contributor Author

b-reyes commented Nov 23, 2022

@emiliom I am done addressing and responding to your comments. This PR is ready for another review.

@emiliom
Copy link
Contributor

emiliom commented Nov 28, 2022

Everything looks good. The only open question is the one about self.survey.params['species_code_ID'] vs self.survey.params['species_id'].

@b-reyes
Copy link
Contributor Author

b-reyes commented Nov 28, 2022

@emiliom I have went ahead and changed the input name species_code_ID to species_id. I believe this is ready for approval or another round of review.

Copy link
Contributor

@emiliom emiliom left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good. Merge away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants