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

Add Pielou's eveness, fix typo in documentation #45

Merged
merged 13 commits into from
Nov 6, 2023

Conversation

PeetoomHeida
Copy link
Contributor

#44 addressed, fixed a typo in the documentation

@PeetoomHeida
Copy link
Contributor Author

Oops, I started writing an example for the Metacommunity constructor, but I realized after making the PR that I didn't finish it. This is related to #43, but I currently don't understand the package well enough to finish the example.

@richardreeve
Copy link
Member

Thanks @PeetoomHeida! Could you reference Pielou's evenness in the docs, and ideally link to another implementation so I can check the code?

@PeetoomHeida
Copy link
Contributor Author

PeetoomHeida commented Apr 5, 2022

I have grabbed the implementation from R vegan, and made a MWE here. I made the function for this package consistent with the others in there, where cols are sites are rows are spp, so be sure to make that switch in matrix structure when checking it in the Julia version. I also fixed the docstrings and added it to the ecology.md docs

Copy link
Member

@richardreeve richardreeve left a comment

Choose a reason for hiding this comment

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

Thanks for this. Sorry for the delay in spotting you'd made the changes. I've made some comments on your edits - I understand why you've switched rows and columns, but I think it just makes things more confusing given we always use them the other way round - in fact you confused yourself at one point! I'll probably have more to say after you've made these changes, but I'm onto it now. Apologies again for the delay.

.vscode/settings.json Outdated Show resolved Hide resolved
Manifest.toml Outdated Show resolved Hide resolved
docs/src/ecology.md Outdated Show resolved Hide resolved
scratch_file.jl Outdated Show resolved Hide resolved
src/Ecology.jl Outdated Show resolved Hide resolved
src/Ecology.jl Outdated Show resolved Hide resolved
src/Metacommunity.jl Outdated Show resolved Hide resolved
src/Metacommunity.jl Outdated Show resolved Hide resolved
src/Metacommunity.jl Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Apr 20, 2022

Pull Request Test Coverage Report for Build 2213794478

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 16 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+4.0%) to 85.942%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Ecology.jl 0 16 0.0%
Totals Coverage Status
Change from base Build 1090744121: 4.0%
Covered Lines: 538
Relevant Lines: 626

💛 - Coveralls

@codecov
Copy link

codecov bot commented Apr 20, 2022

Codecov Report

Merging #45 (9711d54) into dev (41992a6) will decrease coverage by 1.27%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev      #45      +/-   ##
==========================================
- Coverage   82.05%   80.78%   -1.28%     
==========================================
  Files          15       15              
  Lines         613      666      +53     
==========================================
+ Hits          503      538      +35     
- Misses        110      128      +18     
Impacted Files Coverage Δ
src/Diversity.jl 87.50% <ø> (ø)
src/Ecology.jl 78.04% <0.00%> (-21.96%) ⬇️
src/Metacommunity.jl 77.27% <ø> (+1.66%) ⬆️
src/Partition.jl 100.00% <0.00%> (ø)
src/DiversityMeasure.jl 88.32% <0.00%> (+0.53%) ⬆️
src/Phylogenetics.jl 81.13% <0.00%> (+0.73%) ⬆️
src/API.jl 93.75% <0.00%> (+1.15%) ⬆️
src/Types.jl 94.23% <0.00%> (+1.20%) ⬆️
src/Iterators.jl 60.86% <0.00%> (+1.77%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41992a6...9711d54. Read the comment docs.

@PeetoomHeida
Copy link
Contributor Author

Thanks for this. Sorry for the delay in spotting you'd made the changes. I've made some comments on your edits - I understand why you've switched rows and columns, but I think it just makes things more confusing given we always use them the other way round - in fact you confused yourself at one point! I'll probably have more to say after you've made these changes, but I'm onto it now. Apologies again for the delay.

It looks like a bunch of files that I didn't mean to stage commits to got mixed into the PR. I'll sort this out and take a look at the row/column issues as well.

Copy link
Contributor Author

@PeetoomHeida PeetoomHeida left a comment

Choose a reason for hiding this comment

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

Fixed the issues.

@richardreeve
Copy link
Member

Great. However, before I review it you need to restore Project.toml in the root of the package - that defines the package, so you can't delete that! Then we can run the tests and I'll review it again.

@PeetoomHeida
Copy link
Contributor Author

Oh man, thanks for your patience as I get better at making PRs. Hopefully everything is good now, and hopefully the next PR goes smoother! The Project.toml file should be added now

@richardreeve richardreeve merged commit 9711d54 into EcoJulia:dev Nov 6, 2023
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.

3 participants