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

Fixup table #2

Conversation

rhshadrach
Copy link

@rhshadrach rhshadrach commented May 14, 2024

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

This should allow the table to render in GitHub.

| User specification | Concrete dtype | String alias | Note |
|------------------------------------------|---------------------------------------------------------------|---------------------------------------|----------|
| Unspecified (inference) | `StringDtype(storage="pyarrow"\|"python", na_value=np.nan)` | "string" | (1) |
| `StringDtype()` or `"string"` | `StringDtype(storage="pyarrow" \| "python", na_value=np.nan)` | "string" | (1), (2) |

Choose a reason for hiding this comment

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

The problem is that for our online web version, the \ just renders as well and doesn't seem to work as escaping the |.
Now, short term people are mostly looking at this through the github diff, so maybe this is worth it, and I should just remember to switch it back before merging ..

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I see. No problem if you just want to close this then. Reading with the \ in there is still very manageable on the webpage; whereas it is not readable on GitHub. But this is quite minor regardless.

Choose a reason for hiding this comment

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

Yes, that is a good reason for now to merge this!

@jorisvandenbossche jorisvandenbossche merged commit 2c58c4c into jorisvandenbossche:pdep-string-dtype May 14, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants