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

CLR did not use na.rm #663

Merged
merged 1 commit into from
Jun 24, 2024
Merged

CLR did not use na.rm #663

merged 1 commit into from
Jun 24, 2024

Conversation

TuomasBorman
Copy link
Contributor

No description provided.

@TuomasBorman
Copy link
Contributor Author

Fixing issue #661

@jarioksa
Copy link
Contributor

Looks OK and safe, but do not change the DESCRIPTION: we change version numbers only at CRAN releases.

@jarioksa
Copy link
Contributor

At closer look, there seems to be a deeper problem in "clr", "alr" and "rclr". None of these passes decostand(..., na.rm) argument to the corresponding .calc_* function. Therefore user cannot set na.rm action, but the value na.rm = TRUE of all .calc_* functions is de facto hard-coded. What you found out was that this setting was not used in one case, but changing this won't allow users to set na.rm in the decostand call (or vegdist call).

I won't merge this PR because much larger changes are needed to pass decostand and vegdist arguments.

@jarioksa jarioksa closed this Jun 24, 2024
@jarioksa jarioksa reopened this Jun 24, 2024
@jarioksa jarioksa merged commit bdec0e2 into vegandevs:master Jun 24, 2024
14 checks passed
@jarioksa
Copy link
Contributor

I think that the problem is bigger than this one, but your fix will be a part of the solution even if larger changes are needed. Thanks for your contribution!

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