Remove use of `kind` property in `ColumnSchema` by karlhigley · Pull Request #65 · NVIDIA-Merlin/core

karlhigley · 2022-04-05T14:16:13Z

This was intended to normalize Pandas nullable types to their closest corresponding Numpy dtypes, but turns out to break with other numpy dtypes that also define the kind property, so must be reverted.

This was intended to normalize Pandas nullable types to their closest corresponding Numpy dtypes, but turns out to break with other numpy dtypes that also define the `kind` property, so must be reverted.

nvidia-merlin-bot · 2022-04-05T14:17:27Z

Click to view CI Results

GitHub pull request #65 of commit a6a5f3f06a20db8ac79c77616740e4232fad8ef5, no merge conflicts.
Running as SYSTEM
Setting status of a6a5f3f06a20db8ac79c77616740e4232fad8ef5 to PENDING with url https://10.20.13.93:8080/job/merlin_core/17/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_core
using credential ce87ff3c-94f0-400a-8303-cb4acb4918b5
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/core # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/core
 > git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems username and pass
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/core +refs/pull/65/*:refs/remotes/origin/pr/65/* # timeout=10
 > git rev-parse a6a5f3f06a20db8ac79c77616740e4232fad8ef5^{commit} # timeout=10
Checking out Revision a6a5f3f06a20db8ac79c77616740e4232fad8ef5 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f a6a5f3f06a20db8ac79c77616740e4232fad8ef5 # timeout=10
Commit message: "Remove use of `kind` property in `ColumnSchema`"
 > git rev-list --no-walk 693e6776671d703478fe3f25b76732836b7936a8 # timeout=10
[merlin_core] $ /bin/bash /tmp/jenkins1559533346675254086.sh
Looking in indexes: https://pypi.org/simple, https://pypi.ngc.nvidia.com
Requirement already satisfied: setuptools in /usr/local/lib/python3.8/dist-packages (62.0.0)
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.1, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_core/core, configfile: pyproject.toml
plugins: xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 337 items / 1 skipped
tests/unit/core/test_dispatch.py ..                                      [  0%]

tests/unit/dag/test_base_operator.py ....                                [  1%]

tests/unit/dag/test_column_selector.py ..........................        [  9%]

tests/unit/dag/test_tags.py ......                                       [ 11%]

tests/unit/dag/ops/test_selection.py ...                                 [ 12%]

tests/unit/io/test_io.py ............................................... [ 26%]

................................................................         [ 45%]

tests/unit/schema/test_column_schemas.py ............................... [ 54%]

........................................................................ [ 75%]

........................................................................ [ 97%]

[ 97%]

tests/unit/schema/test_schema_io.py ..                                   [ 97%]

tests/unit/utils/test_utils.py ........                                  [100%]
=============================== warnings summary ===============================

tests/unit/dag/test_base_operator.py: 4 warnings

tests/unit/io/test_io.py: 72 warnings

/usr/lib/python3.8/site-packages/cudf/core/dataframe.py:1253: UserWarning: The deep parameter is ignored and is only included for pandas compatibility.

warnings.warn(
tests/unit/io/test_io.py::test_validate_and_regenerate_dataset

/var/jenkins_home/workspace/merlin_core/core/merlin/io/parquet.py:549: DeprecationWarning: 'ParquetDataset.pieces' attribute is deprecated as of pyarrow 5.0.0 and will be removed in a future version. Specify 'use_legacy_dataset=False' while constructing the ParquetDataset, and then use the '.fragments' attribute instead.

paths = [p.path for p in pa_dataset.pieces]
tests/unit/utils/test_utils.py::test_nvt_distributed[True-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 37559 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[True-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 33421 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 32779 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 46591 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 42275 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 37493 instead

warnings.warn(
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

================= 337 passed, 1 skipped, 83 warnings in 52.70s =================

Performing Post build task...

Match found for : : True

Logical operation result is TRUE

Running script  : #!/bin/bash

cd /var/jenkins_home/

CUDA_VISIBLE_DEVICES=1 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/core/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"

[merlin_core] $ /bin/bash /tmp/jenkins8429521466668577506.sh

rjzamora · 2022-04-05T14:18:50Z

merlin/schema/schema.py

-            elif hasattr(self.dtype, "kind"):
-                dtype = np.dtype(self.dtype.kind)


I think we still need to handle string dtypes (the parquet change only handles int/float nullable dtypes):

elif isinstance(self.dtype, pd.StringDtype): dtype = np.dtype("O")

To clarify - We could add a check for pandas string types in the parquet reader, but I believe they are created in other places anyway...

Wait, I thought your change avoided using them?

We avoid creating nullable integer and float dtypes in parquet now, but we weren't creating StringDtype there anyway (I think - I need to check that).

Okay - I think you are right. I missed that some categorical columns are producing StringDtype - I'll try another tweak to the parquet reader and see if we can just drop the block as you are suggesting here.

Okay - It does seem that your original change here works fine if #66 is also merged :)

rjzamora · 2022-04-05T14:55:01Z

LGTM, but it looks like you will need someone with higher merlin privileges to approve as well :)

nvidia-merlin-bot · 2022-04-05T15:55:17Z

Click to view CI Results

GitHub pull request #65 of commit 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9, no merge conflicts.
Running as SYSTEM
Setting status of 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9 to PENDING with url https://10.20.13.93:8080/job/merlin_core/22/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_core
using credential ce87ff3c-94f0-400a-8303-cb4acb4918b5
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/core # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/core
 > git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems username and pass
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/core +refs/pull/65/*:refs/remotes/origin/pr/65/* # timeout=10
 > git rev-parse 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9^{commit} # timeout=10
Checking out Revision 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9 # timeout=10
Commit message: "Merge branch 'main' into fix/schema-dtypes"
 > git rev-list --no-walk f08c57e6dc4972e54b6d77e0e90ab4b1566ac089 # timeout=10
[merlin_core] $ /bin/bash /tmp/jenkins8711348676361416056.sh
Looking in indexes: https://pypi.org/simple, https://pypi.ngc.nvidia.com
Requirement already satisfied: setuptools in /usr/local/lib/python3.8/dist-packages (62.0.0)
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.1, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_core/core, configfile: pyproject.toml
plugins: xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 337 items / 1 skipped
tests/unit/core/test_dispatch.py ..                                      [  0%]

tests/unit/dag/test_base_operator.py ....                                [  1%]

tests/unit/dag/test_column_selector.py ..........................        [  9%]

tests/unit/dag/test_tags.py ......                                       [ 11%]

tests/unit/dag/ops/test_selection.py ...                                 [ 12%]

tests/unit/io/test_io.py ............................................... [ 26%]

................................................................         [ 45%]

tests/unit/schema/test_column_schemas.py ............................... [ 54%]

........................................................................ [ 75%]

........................................................................ [ 97%]

[ 97%]

tests/unit/schema/test_schema_io.py ..                                   [ 97%]

tests/unit/utils/test_utils.py ........                                  [100%]
=============================== warnings summary ===============================

tests/unit/dag/test_base_operator.py: 4 warnings

tests/unit/io/test_io.py: 72 warnings

/usr/lib/python3.8/site-packages/cudf/core/dataframe.py:1253: UserWarning: The deep parameter is ignored and is only included for pandas compatibility.

warnings.warn(
tests/unit/io/test_io.py::test_validate_and_regenerate_dataset

/var/jenkins_home/workspace/merlin_core/core/merlin/io/parquet.py:549: DeprecationWarning: 'ParquetDataset.pieces' attribute is deprecated as of pyarrow 5.0.0 and will be removed in a future version. Specify 'use_legacy_dataset=False' while constructing the ParquetDataset, and then use the '.fragments' attribute instead.

paths = [p.path for p in pa_dataset.pieces]
tests/unit/utils/test_utils.py::test_nvt_distributed[True-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 42797 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[True-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 43919 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 33153 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 42047 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 37533 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 33219 instead

warnings.warn(
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

================= 337 passed, 1 skipped, 83 warnings in 52.89s =================

Performing Post build task...

Match found for : : True

Logical operation result is TRUE

Running script  : #!/bin/bash

cd /var/jenkins_home/

CUDA_VISIBLE_DEVICES=1 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/core/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"

[merlin_core] $ /bin/bash /tmp/jenkins8093608676727783392.sh

nvidia-merlin-bot · 2022-04-05T15:56:18Z

Click to view CI Results

GitHub pull request #65 of commit bb55e2348eb0285e65ec3f59c53c5ab0927f548f, no merge conflicts.
Running as SYSTEM
Setting status of bb55e2348eb0285e65ec3f59c53c5ab0927f548f to PENDING with url https://10.20.13.93:8080/job/merlin_core/23/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_core
using credential ce87ff3c-94f0-400a-8303-cb4acb4918b5
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/core # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/core
 > git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems username and pass
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/core +refs/pull/65/*:refs/remotes/origin/pr/65/* # timeout=10
 > git rev-parse bb55e2348eb0285e65ec3f59c53c5ab0927f548f^{commit} # timeout=10
Checking out Revision bb55e2348eb0285e65ec3f59c53c5ab0927f548f (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f bb55e2348eb0285e65ec3f59c53c5ab0927f548f # timeout=10
Commit message: "Merge branch 'main' into fix/schema-dtypes"
 > git rev-list --no-walk 2e7b4c48a1ae2367c3983e605eda7b4d8868a3f9 # timeout=10
[merlin_core] $ /bin/bash /tmp/jenkins3826142596931303025.sh
Looking in indexes: https://pypi.org/simple, https://pypi.ngc.nvidia.com
Requirement already satisfied: setuptools in /usr/local/lib/python3.8/dist-packages (62.0.0)
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.1, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_core/core, configfile: pyproject.toml
plugins: xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 337 items / 1 skipped
tests/unit/core/test_dispatch.py ..                                      [  0%]

tests/unit/dag/test_base_operator.py ....                                [  1%]

tests/unit/dag/test_column_selector.py ..........................        [  9%]

tests/unit/dag/test_tags.py ......                                       [ 11%]

tests/unit/dag/ops/test_selection.py ...                                 [ 12%]

tests/unit/io/test_io.py ............................................... [ 26%]

................................................................         [ 45%]

tests/unit/schema/test_column_schemas.py ............................... [ 54%]

........................................................................ [ 75%]

........................................................................ [ 97%]

[ 97%]

tests/unit/schema/test_schema_io.py ..                                   [ 97%]

tests/unit/utils/test_utils.py ........                                  [100%]
=============================== warnings summary ===============================

tests/unit/dag/test_base_operator.py: 4 warnings

tests/unit/io/test_io.py: 72 warnings

/usr/lib/python3.8/site-packages/cudf/core/dataframe.py:1253: UserWarning: The deep parameter is ignored and is only included for pandas compatibility.

warnings.warn(
tests/unit/io/test_io.py::test_validate_and_regenerate_dataset

/var/jenkins_home/workspace/merlin_core/core/merlin/io/parquet.py:552: DeprecationWarning: 'ParquetDataset.pieces' attribute is deprecated as of pyarrow 5.0.0 and will be removed in a future version. Specify 'use_legacy_dataset=False' while constructing the ParquetDataset, and then use the '.fragments' attribute instead.

paths = [p.path for p in pa_dataset.pieces]
tests/unit/utils/test_utils.py::test_nvt_distributed[True-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 37321 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[True-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 36467 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 38247 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed[False-False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 35531 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[True]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 46067 instead

warnings.warn(
tests/unit/utils/test_utils.py::test_nvt_distributed_force[False]

/var/jenkins_home/.local/lib/python3.8/site-packages/distributed/node.py:160: UserWarning: Port 8787 is already in use.

Perhaps you already have a cluster running?

Hosting the HTTP server on port 33781 instead

warnings.warn(
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

================= 337 passed, 1 skipped, 83 warnings in 52.38s =================

Performing Post build task...

Match found for : : True

Logical operation result is TRUE

Running script  : #!/bin/bash

cd /var/jenkins_home/

CUDA_VISIBLE_DEVICES=1 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/core/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"

[merlin_core] $ /bin/bash /tmp/jenkins5225084504609862169.sh

github-actions · 2022-04-05T15:56:49Z

Documentation preview

https://nvidia-merlin.github.io/core/review/pr-65

Remove use of kind property in ColumnSchema

a6a5f3f

This was intended to normalize Pandas nullable types to their closest corresponding Numpy dtypes, but turns out to break with other numpy dtypes that also define the `kind` property, so must be reverted.

karlhigley added the bug Something isn't working label Apr 5, 2022

karlhigley added this to the Merlin 22.04 milestone Apr 5, 2022

karlhigley requested review from albert17 and rjzamora April 5, 2022 14:16

karlhigley self-assigned this Apr 5, 2022

rjzamora reviewed Apr 5, 2022

View reviewed changes

rjzamora approved these changes Apr 5, 2022

View reviewed changes

albert17 added 2 commits April 5, 2022 08:54

Merge branch 'main' into fix/schema-dtypes

2e7b4c4

Merge branch 'main' into fix/schema-dtypes

bb55e23

albert17 approved these changes Apr 5, 2022

View reviewed changes

albert17 merged commit 4c9fdfa into NVIDIA-Merlin:main Apr 5, 2022

rjzamora mentioned this pull request Apr 5, 2022

Add pd.StringDtype check back to schema __post_init__ #67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove use of `kind` property in `ColumnSchema`#65

Remove use of `kind` property in `ColumnSchema`#65
albert17 merged 3 commits intoNVIDIA-Merlin:mainfrom
karlhigley:fix/schema-dtypes

karlhigley commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Uh oh!

karlhigley Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Uh oh!

rjzamora commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

github-actions bot commented Apr 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		elif hasattr(self.dtype, "kind"):
		dtype = np.dtype(self.dtype.kind)

Conversation

karlhigley commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

rjzamora Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rjzamora Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

karlhigley Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rjzamora Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rjzamora Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rjzamora Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rjzamora commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

nvidia-merlin-bot commented Apr 5, 2022

Uh oh!

github-actions bot commented Apr 5, 2022

Documentation preview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants