You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/09 07:36:09 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #4730: Python: Add spellcheck to the CI
Fokko opened a new pull request, #4730:
URL: https://github.com/apache/iceberg/pull/4730
Simple spellcheck so we don't have to spend any mental energy on the spelling
--
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 #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r872645196
##########
python/src/iceberg/table/partitioning.py:
##########
@@ -24,7 +24,7 @@ class PartitionField:
Attributes:
source_id(int): The source column id of table's schema
- field_id(int): The partition field id across all the table metadata's partition specs
+ field_id(int): The partition field id across all the table partition specs' metadata
Review Comment:
Love it, less is more 👍🏻
--
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 #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r872644783
##########
python/tests/test_schema.py:
##########
@@ -228,7 +228,7 @@ def test_schema_find_field_by_id_raise_on_unknown_field(table_schema_simple):
def test_schema_find_field_type_by_id(table_schema_simple):
- """Test retrieving a columns's type using its field ID"""
+ """Test retrieving a column type using its field ID"""
Review Comment:
Updated!
--
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 #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868349447
##########
python/src/iceberg/table/partitioning.py:
##########
@@ -24,7 +24,7 @@ class PartitionField:
Attributes:
source_id(int): The source column id of table's schema
- field_id(int): The partition field id across all the table metadata's partition specs
+ field_id(int): The partition field id across all the table partition metadata specs
Review Comment:
I've updated it to `The partition field id across all the table partition specs metadata` WDYT?
--
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] kbendick commented on a diff in pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r870775434
##########
python/tox.ini:
##########
@@ -52,10 +52,13 @@ deps =
black
isort
autoflake
+ pylint
+ pyenchant
commands =
autoflake -r --check --ignore-init-module-imports --remove-all-unused-imports src tests
isort --profile black --check-only src tests
black --line-length 130 --check --diff src tests
+ pylint --disable all --enable spelling --spelling-dict en_US --spelling-private-dict-file spellcheck-dictionary.txt en_US src tests
Review Comment:
Question / Nit: Do we need to specify `en_US` twice? Specifically for the 2nd argument after `--spelling-private-dict-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] rdblue commented on a diff in pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868748459
##########
dev/.rat-excludes:
##########
@@ -25,4 +25,5 @@ package-list
sitemap.xml
derby.log
.python-version
+spellcheck-dictionary.txt
Review Comment:
There's no way to add comments to this 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] rdblue commented on a diff in pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868750132
##########
python/tests/test_schema.py:
##########
@@ -228,7 +228,7 @@ def test_schema_find_field_by_id_raise_on_unknown_field(table_schema_simple):
def test_schema_find_field_type_by_id(table_schema_simple):
- """Test retrieving a columns's type using its field ID"""
+ """Test retrieving a column type using its field ID"""
Review Comment:
"column's"? I think the possessive form should still be used.
--
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] rdblue merged pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #4730:
URL: https://github.com/apache/iceberg/pull/4730
--
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 #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868352290
##########
python/spellcheck-dictionary.txt:
##########
@@ -0,0 +1,33 @@
+accessor
Review Comment:
> just curious, do we need to manually maintain the private dictionary going forward?
Yes, once there is special jargon that's outside of the English dictionary, we need to add this.
> also I am curious, do we need both python/spellcheck-dictionary.txt and top level spellcheck-dictionary.txt?
Yes, in that case, we would have another dictionary, but a better option would be to move it to the top level. Or in `dev/` to keep the top-level nice and clean.
--
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] rdblue commented on pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#issuecomment-1126568335
Thanks, @Fokko!
--
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 #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r872644479
##########
python/tox.ini:
##########
@@ -52,10 +52,13 @@ deps =
black
isort
autoflake
+ pylint
+ pyenchant
commands =
autoflake -r --check --ignore-init-module-imports --remove-all-unused-imports src tests
isort --profile black --check-only src tests
black --line-length 130 --check --diff src tests
+ pylint --disable all --enable spelling --spelling-dict en_US --spelling-private-dict-file spellcheck-dictionary.txt en_US src tests
Review Comment:
Great catch @kbendick! That's just me being messy with copy and paste!
--
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] rdblue commented on a diff in pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868749741
##########
python/src/iceberg/table/partitioning.py:
##########
@@ -24,7 +24,7 @@ class PartitionField:
Attributes:
source_id(int): The source column id of table's schema
- field_id(int): The partition field id across all the table metadata's partition specs
+ field_id(int): The partition field id across all the table partition specs' metadata
Review Comment:
This changes the meaning. "Table metadata" refers to all of the metadata, and "table metadata's partition specs" is the set of partition specs in table metadata. "Partition specs' metadata" is narrower, the metadata across partition specs.
To fix this, I think you can just omit "metadata": "across all the table's partition specs".
--
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] dramaticlly commented on a diff in pull request #4730: Python: Add spellcheck to the CI
Posted by GitBox <gi...@apache.org>.
dramaticlly commented on code in PR #4730:
URL: https://github.com/apache/iceberg/pull/4730#discussion_r868221402
##########
python/src/iceberg/table/partitioning.py:
##########
@@ -24,7 +24,7 @@ class PartitionField:
Attributes:
source_id(int): The source column id of table's schema
- field_id(int): The partition field id across all the table metadata's partition specs
+ field_id(int): The partition field id across all the table partition metadata specs
Review Comment:
nit: I think [partition spec](https://iceberg.apache.org/spec/#partition-specs) here refer to a single entity in iceberg jargon, so I feel `partition metadata specs` does not confer the same meaning
##########
python/spellcheck-dictionary.txt:
##########
@@ -0,0 +1,33 @@
+accessor
Review Comment:
just curious, do we need to manually maintain the private dictionary going forward?
##########
python/spellcheck-dictionary.txt:
##########
@@ -0,0 +1,33 @@
+accessor
Review Comment:
also I am curious, do we need both `python/spellcheck-dictionary.txt` and top level `spellcheck-dictionary.txt`?
--
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