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(colformat_md): follow internal change of flextable data structure #101

Merged
merged 5 commits into from Mar 31, 2024

Conversation

atusy
Copy link
Owner

@atusy atusy commented Mar 30, 2024

fixes #100

R/colformat.R Outdated
Comment on lines 54 to 59
content <- if (is.null(x[[part]][["content"]][["content"]])) {
# flextable >= 0.9.5
x[[part]][["content"]][["data"]]
} else {
x[[part]][["content"]][["content"]][["data"]]
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

@davidgohel The issue #100 seems to be happened because of the change in the internal data structure. Am I right? (I have not yet reviewed the git log)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes the internal data structure has been simplified. Sorry I ran revdepcheck and did not catch that issue

@atusy
Copy link
Owner Author

atusy commented Mar 31, 2024

I will drop support for flextable < 0.9.5 by specifying required version in DESCRIPTION

Copy link

cloudflare-pages bot commented Mar 31, 2024

Deploying ftextra with  Cloudflare Pages  Cloudflare Pages

Latest commit: 236f205
Status: ✅  Deploy successful!
Preview URL: https://8b2c1ab3.ftextra.pages.dev
Branch Preview URL: https://support-flextable-0-9-5.ftextra.pages.dev

View logs

@davidgohel
Copy link
Contributor

davidgohel commented Mar 31, 2024

@atusy is it possible to use exported functions (I. E. as_paragraph or append_chunk) instead of using internal structures?

That would help having more robust code for both of us, see for example https://github.com/davidgohel/flextable/blob/2784247f5b500eca8fe4f9983326bf17c3897d12/R/01_fpstruct.R#L313

@atusy
Copy link
Owner Author

atusy commented Mar 31, 2024

@davidgohel

Thanks. I understand robust code should avoid relying on the internal data structure.
However, I find no exported functions suits my use cases...

  1. To convert texts in selected columns, I need access to the internal data to retrieve target markdown texts...
  2. colformat_md's j argument supports the tidy-selection. This also requires access to the internal data structure... I like the experience of the tidy-selection, but I regret the inconsistency with the other colformat functions in the original flextable package.

To solve both, I need flextable to export a function that provides access to the internal data structure. However, but I do not think this is a good idea because flextable may face a difficulty to change the internal data structure in the future.

To solve the first one, I have a suggestion. flextable::compose()'s value argument can be enhanced to accept a callback that receives a paragraph selected by x, i, and j arguments, and returns the modified paragraph.
This also enables users to modify the content programmatically.

Anyway, I would be happy to know if there is a way for ftExtra to avoid the dependency on the internal data.
Thanks again.

@davidgohel
Copy link
Contributor

Thanks for the suggestion 😊
I don't have a computer right now to try your suggestion but I will have a try later this week and iterate on the subject in this thread

@atusy
Copy link
Owner Author

atusy commented Mar 31, 2024

My pleasure!

I would like to release a fix as soon as possible, so let me continue relying on the internal data structure for now.

Maybe I can do better in the near future 😄

@atusy atusy merged commit 30d5d70 into master Mar 31, 2024
6 checks passed
@atusy atusy deleted the support-flextable-0-9-5 branch March 31, 2024 12:31
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.

colformat_md() not working with flextable version 0.9.5
2 participants