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: names in lavaan_extract #36

Merged
merged 2 commits into from
Jun 19, 2024
Merged

fix: names in lavaan_extract #36

merged 2 commits into from
Jun 19, 2024

Conversation

ethanbass
Copy link
Contributor

@ethanbass ethanbass commented Jun 19, 2024

Hi,
I think the lavaan_extract function only works as intended currently if you provide a list of replacement symbols rather than just one. Otherwise, the function overwrites all the paths in column one with whatever's in the first row as you can see in this screenshot from the vignette:

image

The current version works correctly only if you provide a vector of replacement characters, e.g.,
lavaanExtra::lavaan_defined(fit,nice_table = TRUE, underscores_to_symbol = rep("->",4))

With my fix, it displays correctly with the default syntax:
lavaanExtra::lavaan_defined(fit,nice_table = TRUE, underscores_to_symbol = "->")

image

Thanks!
Ethan

@rempsyc
Copy link
Owner

rempsyc commented Jun 19, 2024

Amazing, thank you for signaling this bug and for fixing it in this PR!

The tests are only failing because the tests snapshots have changed following this change. Do you know how to update snapshots from your fork? If so could you please update the snapshots and then I will merge your PR. Thanks!

@rempsyc rempsyc added the bug Something isn't working label Jun 19, 2024
@rempsyc rempsyc self-requested a review June 19, 2024 15:14
@ethanbass
Copy link
Contributor Author

No problem! I tried updating the snapshots. I can't see the tests on the CI, but it passed on my computer just now.
Ethan

@ethanbass
Copy link
Contributor Author

ethanbass commented Jun 19, 2024

Still failing on mac (and windows) apparently, which is strange because I updated the snapshots on my mac and the tests passed locally. any ideas?

@rempsyc rempsyc merged commit 8d3e0aa into rempsyc:main Jun 19, 2024
8 of 10 checks passed
@rempsyc
Copy link
Owner

rempsyc commented Jun 19, 2024

Thank you for your contribution @ethanbass ! 😄 Hope you enjoy the package 😊

@ethanbass
Copy link
Contributor Author

Thanks, it seems really helpful so far!

@rempsyc
Copy link
Owner

rempsyc commented Jun 19, 2024

All tests are passing now 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants