[CI] Fix some warnings #4547

lhoestq · 2022-06-23T10:10:49Z

There are some warnings in the CI that are annoying, I tried to remove most of them

HuggingFaceDocBuilderDev · 2022-06-23T10:19:56Z

The documentation is not available anymore as the PR was closed or merged.

albertvillanova

Thanks a lot: great to get rid off all warning messages!!!

albertvillanova · 2022-06-24T09:56:12Z

src/datasets/arrow_dataset.py

                    np_arrays.append(np.array(array))

-            if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == np.bool:
+            if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == np.dtype("bool"):


The warning message suggests to use bool instead.

Suggested change

if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == np.dtype("bool"):

if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == bool:

Done, and for object as well

albertvillanova · 2022-06-24T10:01:40Z

src/datasets/commands/datasets_cli.py

 from datasets.commands.env import EnvironmentCommand
 from datasets.commands.run_beam import RunBeamCommand
-from datasets.commands.test import TestCommand
+from datasets.commands.test import CLITestCommand


You are preceding with CLI only the TestCommand; i.e. the rest of the command names are not preceded by CLI.

IMHO, either all or none should be preceded by CLI.

On the other hand, from your commit message, I guess you introduce this modification to prevent pytest default test discovery to identify this class as a test. If this is the reason, I would strongly suggest using testpaths instead: https://docs.pytest.org/en/latest/reference/reference.html#confval-testpaths

~~We tell pytest to search for tests only inside the tests directory~~

~~Moreover, tests discovery/collection will be faster~~

I just realized pytest is called with arg ./tests... I did not consider imported classes as possible test candidates, thus I thought pytest was called without arguments and it was searching inside src to find TestCommand. Sorry for the confusion.

pytest suggests adding the class attribute __test__ = False to TestCommand, but I am not sure if this is better... :/

I went for __test__ = False since it's simpler

src/datasets/commands/datasets_cli.py

src/datasets/commands/test.py

albertvillanova · 2022-06-24T10:23:14Z

src/datasets/formatting/tf_formatter.py

+            if data_struct.dtype == np.dtype(
+                "object"


Same as above.

Suggested change

if data_struct.dtype == np.dtype(

"object"

if (

data_struct.dtype == object

This is not resolved yet.

albertvillanova · 2022-06-24T10:23:53Z

src/datasets/formatting/torch_formatter.py

+            if data_struct.dtype == np.dtype(
+                "object"
+            ):  # pytorch tensors cannot be instantied from an array of objects


Same.

Suggested change

if data_struct.dtype == np.dtype(

"object"

): # pytorch tensors cannot be instantied from an array of objects

if data_struct.dtype == object: # pytorch tensors cannot be instantied from an array of objects

This is not resolved yet.

tests/commands/test_test.py

tests/features/test_array_xd.py

This reverts commit 877c7a9.

lhoestq · 2022-06-28T10:32:40Z

There is a CI failure only related to the missing content of the universal_dependencies dataset card, we can ignore this failure in this PR

albertvillanova

Just some leftovers to be fixed before merging.

albertvillanova · 2022-06-28T13:07:56Z

src/datasets/commands/datasets_cli.py

 from datasets.commands.env import EnvironmentCommand
 from datasets.commands.run_beam import RunBeamCommand
-from datasets.commands.test import TestCommand
+from datasets.commands.test import CLITestCommand


Please note there are still some CLI... leftovers you should fix before merging.

albertvillanova · 2022-06-28T13:08:06Z

src/datasets/commands/datasets_cli.py

    ConvertCommand.register_subcommand(commands_parser)
    EnvironmentCommand.register_subcommand(commands_parser)
-    TestCommand.register_subcommand(commands_parser)
+    CLITestCommand.register_subcommand(commands_parser)


Same as above...

albertvillanova · 2022-06-28T13:09:32Z

src/datasets/formatting/tf_formatter.py

+            if data_struct.dtype == np.dtype(
+                "object"


This is not resolved yet.

albertvillanova · 2022-06-28T13:09:49Z

src/datasets/formatting/torch_formatter.py

+            if data_struct.dtype == np.dtype(
+                "object"
+            ):  # pytorch tensors cannot be instantied from an array of objects


This is not resolved yet.

lhoestq · 2022-06-28T13:26:33Z

good catch, I thought I resolved them all sorry

lhoestq · 2022-06-28T13:28:51Z

Alright it should be good now

albertvillanova

Thank you!!!

lhoestq added 10 commits June 23, 2022 12:08

fix bad escape chars

00ce68b

fix np.bool

5541c57

fix np.object

2ca3620

don't name class or funcs like test_*

877c7a9

fix np.int

ab17252

fix name -> repo_id in hfh

3041959

fix name -> config_name in builder

bcdb64a

don't name class or func like test_* again

305c6b4

fix np.object again

4cd685b

style

6841ebc

lhoestq added 3 commits June 23, 2022 14:05

Merge branch 'master' into fix-some-warnings

6cdcee2

fix universal_dependencies languages

899e544

fix bad import

5741988

albertvillanova reviewed Jun 24, 2022

View reviewed changes

lhoestq added 5 commits June 28, 2022 12:18

Merge branch 'master' into fix-some-warnings

020a18b

Revert "don't name class or funcs like test_*"

031e8d0

This reverts commit 877c7a9.

use bool

ec7a245

use object

26cb868

use __test__ = False

21769e2

fix test

03a81b4

albertvillanova requested changes Jun 28, 2022

View reviewed changes

fix remaining changes

3237c8d

albertvillanova approved these changes Jun 28, 2022

View reviewed changes

lhoestq merged commit f85a78d into master Jun 28, 2022

lhoestq deleted the fix-some-warnings branch June 28, 2022 13:59

	if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == np.dtype("bool"):
	if np.issubdtype(np_arrays[0].dtype, np.integer) or np_arrays[0].dtype == bool:

[CI] Fix some warnings #4547

[CI] Fix some warnings #4547

Uh oh!

Conversation

lhoestq commented Jun 23, 2022

Uh oh!

HuggingFaceDocBuilderDev commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

albertvillanova left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albertvillanova Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albertvillanova Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhoestq commented Jun 28, 2022

Uh oh!

albertvillanova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhoestq commented Jun 28, 2022

Uh oh!

lhoestq commented Jun 28, 2022

Uh oh!

albertvillanova left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HuggingFaceDocBuilderDev commented Jun 23, 2022 •

edited

Loading

albertvillanova left a comment •

edited

Loading

albertvillanova Jun 24, 2022 •

edited

Loading

albertvillanova Jun 24, 2022 •

edited

Loading