Skip to content

Python: Use Prettier to format TOML and more files#7248

Merged
Fokko merged 3 commits into
apache:masterfrom
deepyaman:python/prettier
Apr 8, 2023
Merged

Python: Use Prettier to format TOML and more files#7248
Fokko merged 3 commits into
apache:masterfrom
deepyaman:python/prettier

Conversation

@deepyaman
Copy link
Copy Markdown
Contributor

Came across #6745 (comment) while looking into something else, and figured I could do it quickly. :)

Prettier is a well-established auto-formatter for many types of files. I've personally used it on a lot of projects int he past. The TOML integration is community-built (but linked to from the official Prettier website and docs) and no longer maintained, but still seems to function fine (including in testing on the case from the aforementioned comment). On the whole, it did seem to help standardize formatting of the pyproject.toml file.

Copy link
Copy Markdown
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Thanks @deepyaman, this looks great! I have one comment about the note, would it be possible to have to linker accept that? Otherwise, it breaks the note.

Comment thread python/mkdocs/docs/api.md Outdated
!!! note "Requirements"
This requires [PyArrow to be installed](index.md)

This requires [PyArrow to be installed](index.md)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This breaks the note unfortunately.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

image

😭

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm... I haven't used mkdocs previously, but it seems like their admonition syntax isn't supported by Prettier (or the underlying parser). There are a number of existing issues (e.g. prettier/prettier#3362 and prettier/prettier#12985) highlighting this, and the recommendation seems to wrap them in <!-- prettier-ignore-start -->, <!-- prettier-ignore-end -->. I can do that if it's acceptable?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm aware of the note syntax being a bit odd indeed, other linters are also having issues with that. I'm okay with the ignore comments 👍🏻

Copy link
Copy Markdown
Contributor Author

@deepyaman deepyaman Apr 3, 2023

Choose a reason for hiding this comment

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

@Fokko Sorry for the delay, but I think it looks good now!

image

| Create Namespace | X | X |
| Drop Namespace | X | X |
| Set Namespace Properties | X | X |
| Operation | Java | Python |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked and this looks good:
image

Copy link
Copy Markdown
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

@deepyaman Love it, thanks for picking this up!

@Fokko Fokko merged commit 3afa71a into apache:master Apr 8, 2023
ericlgoodman pushed a commit to ericlgoodman/iceberg that referenced this pull request Apr 12, 2023
* Python: Use Prettier to format TOML and more files

* Python: Make Prettier skip the mkdocs admonoitions
@deepyaman deepyaman deleted the python/prettier branch April 25, 2023 12:20
manisin pushed a commit to Snowflake-Labs/iceberg that referenced this pull request May 9, 2023
* Python: Use Prettier to format TOML and more files

* Python: Make Prettier skip the mkdocs admonoitions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants