You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "deepyaman (via GitHub)" <gi...@apache.org> on 2023/03/31 02:06:58 UTC
[GitHub] [iceberg] deepyaman opened a new pull request, #7248: Python: Use Prettier to format TOML and more files
deepyaman opened a new pull request, #7248:
URL: https://github.com/apache/iceberg/pull/7248
Came across https://github.com/apache/iceberg/pull/6745#discussion_r1100602879 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] deepyaman commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "deepyaman (via GitHub)" <gi...@apache.org>.
deepyaman commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1156184882
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
@Fokko Sorry for the delay, but I think it looks good now!
<img width="1436" alt="image" src="https://user-images.githubusercontent.com/14007150/230460823-3c93e62c-42e2-4c93-ba57-80e33fca6ed0.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1154371568
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
😭
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1154454657
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
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 👍🏻
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] deepyaman commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "deepyaman (via GitHub)" <gi...@apache.org>.
deepyaman commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1154386270
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
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. https://github.com/prettier/prettier/issues/3362 and https://github.com/prettier/prettier/issues/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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] deepyaman commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "deepyaman (via GitHub)" <gi...@apache.org>.
deepyaman commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1156184882
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
@Fokko Sorry for the delay, but I think it looks good now!
![Uploading image.png…]()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7248:
URL: https://github.com/apache/iceberg/pull/7248#discussion_r1154367480
##########
python/mkdocs/docs/api.md:
##########
@@ -307,15 +307,18 @@ scan = table.scan(
The low level API `plan_files` methods returns a set of tasks that provide the files that might contain matching rows:
```json
-['s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet']
+[
+ "s3a://warehouse/wh/nyc/taxis/data/00003-4-42464649-92dd-41ad-b83b-dea1a2fe4b58-00001.parquet"
+]
```
In this case it is up to the engine itself to filter the file itself. Below, `to_arrow()` and `to_duckdb()` that already do this for you.
### Apache Arrow
!!! note "Requirements"
- This requires [PyArrow to be installed](index.md)
+
+This requires [PyArrow to be installed](index.md)
Review Comment:
This breaks the note unfortunately.
##########
python/mkdocs/docs/feature-support.md:
##########
@@ -21,21 +21,21 @@ The goal is that the python library will provide a functional, performant subset
## Metadata
-| Operation | Java | Python |
-|:-------------------------|:-----:|:------:|
-| Get Schema | X | X |
-| Get Snapshots | X | X |
-| Plan Scan | X | X |
-| Plan Scan for Snapshot | X | X |
-| Update Current Snapshot | X | |
-| Create Table | X | X |
-| Rename Table | X | X |
-| Drop Table | X | X |
-| Alter Table | X | |
-| Set Table Properties | X | |
-| Create Namespace | X | X |
-| Drop Namespace | X | X |
-| Set Namespace Properties | X | X |
+| Operation | Java | Python |
Review Comment:
I checked and this looks good:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko merged pull request #7248: Python: Use Prettier to format TOML and more files
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #7248:
URL: https://github.com/apache/iceberg/pull/7248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org