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

Rename normalized_chi() to fei() #475

Merged
merged 14 commits into from
Sep 12, 2022
Merged

Rename normalized_chi() to fei() #475

merged 14 commits into from
Sep 12, 2022

Conversation

mattansb
Copy link
Member

Closes #474

The פ should render fine in the docs, right?

@mattansb mattansb requested a review from bwiernik September 10, 2022 16:48
@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2022

Codecov Report

Merging #475 (d1a25fd) into main (b9d3149) will decrease coverage by 0.12%.
The diff coverage is 67.85%.

@@            Coverage Diff             @@
##             main     #475      +/-   ##
==========================================
- Coverage   90.28%   90.16%   -0.13%     
==========================================
  Files          48       48              
  Lines        2759     2765       +6     
==========================================
+ Hits         2491     2493       +2     
- Misses        268      272       +4     
Impacted Files Coverage Δ
R/effectsize.R 96.77% <ø> (ø)
R/is_effectsize_name.R 100.00% <ø> (ø)
R/zzz_deprecated.R 0.00% <0.00%> (ø)
R/xtab.R 79.18% <70.58%> (ø)
R/convert_stat_chisq.R 89.18% <100.00%> (ø)
R/effectsize.htest.R 96.89% <100.00%> (+0.01%) ⬆️
R/interpret.R 90.24% <100.00%> (+0.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mattansb
Copy link
Member Author

Hmm... I'll take that as a "no".

Any ideas @bwiernik @strengejacke @IndrajeetPatil ?

❯ checking PDF version of manual ... WARNING
  LaTeX errors when creating PDF version.
  This typically indicates Rd problems.
  LaTeX errors found:
  ! Package inputenc Error: Unicode character פ (U+05E4)
  (inputenc)                not set up for use with LaTeX.

@bwiernik
Copy link
Contributor

bwiernik commented Sep 10, 2022

Does using the \u Unicode escape work? Or what if you put it in an equation?

@mattansb
Copy link
Member Author

mattansb commented Sep 10, 2022

Currently it's always in \eqn{פ}.
I'm gonna replace it with XXX just to see if all of the errors are related to that.

@mattansb
Copy link
Member Author

  • ubuntu-latest (release) + ubuntu-latest (devel) fails due to not being able to build XML
  • ubuntu-latest (oldrel-2) + ubuntu-latest (oldrel-3) are failing due to running code in examples that is in \donttest{}.

@IndrajeetPatil, any idea why these are happening?

@IndrajeetPatil
Copy link
Member

This is beyond our control, unfortunately.

r-lib/actions#559

@mattansb
Copy link
Member Author

Ah okay :/

Any idea why an example is running in \donttest{}? (I hate these tags...)

@bwiernik
Copy link
Contributor

CRAN tests examples in donttest (for some inexplicable reason). If you want them not to run ever, use dontrun

@bwiernik
Copy link
Contributor

bwiernik commented Sep 11, 2022

You can't use the actual symbol. Only ASCII is allowed on CRAN. You need to use latex macros in eqn or the Unicode escape numbers. For the PDF, it might be the case that we just can't use the symbols if there isn't a built in latex macro we can use in eqn. In that case, have it conditionally display the symbol in HTML and plain text (via the Unicode escape) and just the fei spelling in Latex. (Because very few people read the pdf manual and CRAN's support for TeX is so limited)

@mattansb
Copy link
Member Author

I'm having some problems with the escape numbers - they don't seem to be doing anything:

Convert between Chi square (\eqn{\chi^2}), phi (\eqn{\phi}), Cramer's V,
Cohen's *w*, Fei (\u05e4) and Pearson's *C* for contingency tables or
goodness of fit.

Is just rendered as:

image

What am I doing wrong? 🤔

@bwiernik
Copy link
Contributor

I'll try it out

@bwiernik
Copy link
Contributor

The Writing R Extensions docs make it sound like including UTF-8 characters in the Latex version should be possible (https://cran.r-project.org/doc/manuals/R-exts.html#Encoding), but I can't figure out what to do there.

So, for now, I just set it to use the proper symbols in HTML and text, but Fei in PDF (ironic considering that Writing R Extensions says "Mathematical formulae should be set beautifully for printed [PDF] documentation")

@mattansb
Copy link
Member Author

The Writing R Extensions docs make it sound like including UTF-8 characters in the Latex version should be possible (https://cran.r-project.org/doc/manuals/R-exts.html#Encoding), but I can't figure out what to do there.

Yup - I also saw some examples on SO, but could not make them work :/

Thanks B!

Once I see all tests pass now on my latest push I will merge.

Welcome Fei!

@mattansb mattansb merged commit f7f2409 into main Sep 12, 2022
@mattansb mattansb deleted the rename-chinorm branch September 12, 2022 18:34
@bwiernik
Copy link
Contributor

Can you share the links to the StackOverflow examples?

@bwiernik
Copy link
Contributor

welcome Fei!

The title of the paper should totally start with "phi, fei, fo, fum…"

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.

Rename normalized Chi to Waw / Fei?
4 participants