Repository: glotzerlab/signac
Pull request #692 (2022-02-26 05:57:09)

Use `FutureWarning` instead of `DeprecationWarning`.
<!-- Provide a general summary of your changes in the Title above -->

## Description
<!-- Describe your changes in detail -->
Fixes issues as described by @vyasr in Issue https://github.com/glotzerlab/signac/issues/687 as we tend to change the DeprecationWarning to FutureWarning instead.
## Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
The main motivation of this change is basically that warnings of the deprecated features are visible to the users as in current scenarios of Deprecated warnings python hides these by default.

## Types of Changes
<!-- Please select all items that apply either now or after creating the pull request: -->
- [ ] Documentation update
- [ ] Bug fix
- [x] New feature
- [ ] Breaking change<sup>1</sup>

<sup>1</sup>The change breaks (or has the potential to break) existing functionality.

## Checklist:
<!-- Please select all items that apply either now or after creating the pull request. -->
<!-- If you are unsure about any of these items, do not hesitate to ask! -->
- [x] I am familiar with the [**Contributing Guidelines**](https://github.com/glotzerlab/signac/blob/master/CONTRIBUTING.md).
- [x] I agree with the terms of the [**Contributor Agreement**](https://github.com/glotzerlab/signac/blob/master/ContributorAgreement.md).
- [x] My name is on the [list of contributors](https://github.com/glotzerlab/signac/blob/master/contributors.yaml).
- [x] My code follows the [code style guideline](https://github.com/glotzerlab/signac/blob/master/CONTRIBUTING.md#code-style) of this project.
- [x] The changes introduced by this pull request are covered by existing or newly introduced tests.

If necessary:
- [ ] I have updated the API documentation as part of the package doc-strings.
- [ ] I have created a separate pull request to update the [framework documentation](https://docs.signac.io/) on [signac-docs](https://github.com/glotzerlab/signac-docs) and linked it here.
- [ ] I have updated the [changelog](https://github.com/glotzerlab/signac/blob/master/changelog.txt) and added all related issue and pull request numbers for future reference (if applicable). See example below.

<!-- Example for a changelog entry: `Fix issue with launching rockets to the moon (#101, #212).` -->


codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) Report
> Merging [#692](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (234c34e) into [master](https://codecov.io/gh/glotzerlab/signac/commit/c291e1ef1d8cc7bb3b9591ae4e6265f4c27cf7aa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (c291e1e) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/glotzerlab/signac/pull/692/graphs/tree.svg?width=650&height=150&src=pr&token=FghbjTyzgM&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)

```diff
@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
+ Coverage   78.53%   78.54%   +0.01%     
==========================================
  Files          65       65              
  Lines        7141     7141              
  Branches     1565     1565              
==========================================
+ Hits         5608     5609       +1     
  Misses       1228     1228              
+ Partials      305      304       -1     
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | Coverage Δ | |
|---|---|---|
| [signac/contrib/indexing.py](https://codecov.io/gh/glotzerlab/signac/pull/692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvaW5kZXhpbmcucHk=) | `70.95% <ø> (ø)` | |
| [signac/\_\_main\_\_.py](https://codecov.io/gh/glotzerlab/signac/pull/692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19fbWFpbl9fLnB5) | `71.89% <0.00%> (+0.11%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Last update [c291e1e...234c34e](https://codecov.io/gh/glotzerlab/signac/pull/692?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab).

lyrivera: Not sure what to review without a description.
vyasr: @Onkar627 when you address the reviews could you also fill out the PR template? You can see the sections: 1) a description of your changes; 2) an explanation of why you are making these changes (the motivation); and 3) filling out the checklist.
Onkar627: > @Onkar627 when you address the reviews could you also fill out the PR template? You can see the sections: 1) a description of your changes; 2) an explanation of why you are making these changes (the motivation); and 3) filling out the checklist.

Yes sure, As I changed it for other pull request will do the same for this as well.
Onkar627: @bdice , @lyrivera I have updated the PR, kindly review and please ping if any changes are required.
bdice: @Onkar627 There are some tests failing. See logs [here for linux-python-38](https://app.circleci.com/pipelines/github/glotzerlab/signac/2806/workflows/8a430ce8-61fc-472c-bef0-dbd49e107332/jobs/19395).

The underlying cause is that we are using a pytest feature `pytest.deprecated_call` to catch deprecation warnings, but that does not catch `FutureWarning`. We need to replace some code in the places where tests are failing to use `pytest.warns` instead. For example, this line should be changed in `tests/test_synced_collections/test_json_buffered_collection.py`:
```diff
- with pytest.deprecated_call(match="Use of.+as key is deprecated"):
+ with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"):
```
Onkar627: > @Onkar627 There are some tests failing. See logs [here for linux-python-38](https://app.circleci.com/pipelines/github/glotzerlab/signac/2806/workflows/8a430ce8-61fc-472c-bef0-dbd49e107332/jobs/19395).
> 
> The underlying cause is that we are using a pytest feature `pytest.deprecated_call` to catch `DeprecationWarning`, but that does not catch `FutureWarning`. We need to replace some code in the places where tests are failing to use `pytest.warns` instead. For example, this line should be changed in `tests/test_synced_collections/test_json_buffered_collection.py`:
> 
> ```diff
> - with pytest.deprecated_call(match="Use of.+as key is deprecated"):
> + with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"):
> ```

Should I also change every deprecated call feature to warn like for example in  `tests/conftest.py`:
> 
> ```diff
> def deprecated_in_version(version_string):
>    if version.parse(version_string) <= version.parse(signac.__version__):
> - with pytest.deprecated_call():
> + with pytest.warns(FutureWarning):
> ```
@bdice 

Patch:
diff --git a/signac/contrib/indexing.py b/signac/contrib/indexing.py
index 0ca4b83e..e2c70e81 100644
--- a/signac/contrib/indexing.py
+++ b/signac/contrib/indexing.py
@@ -620,7 +620,7 @@ class MasterCrawler(MainCrawler):
         warnings.warn(
             "The MasterCrawler class has been replaced by the MainCrawler class. "
             "Both classes are deprecated and will be removed in a future release.",
-            DeprecationWarning,
+            FutureWarning,
         )
         super().__init__(*args, **kwargs)
 
diff --git a/signac/synced_collections/backends/collection_json.py b/signac/synced_collections/backends/collection_json.py
index ce4caa65..ca2b8fc0 100644
--- a/signac/synced_collections/backends/collection_json.py
+++ b/signac/synced_collections/backends/collection_json.py
@@ -49,7 +49,7 @@ def _str_key(key):
         warnings.warn(
             f"Use of {type(key).__name__} as key is deprecated "
             "and will be removed in version 2.0",
-            DeprecationWarning,
+            FutureWarning,
         )
         key = str(key)
     return key
@@ -146,7 +146,7 @@ def json_attr_dict_validator(data):
                 warnings.warn(
                     f"Use of {type(key).__name__} as key is deprecated "
                     "and will be removed in version 2.0.",
-                    DeprecationWarning,
+                    FutureWarning,
                 )
                 data[str(key)] = data.pop(key)
             else:
diff --git a/signac/synced_collections/buffers/file_buffered_collection.py b/signac/synced_collections/buffers/file_buffered_collection.py
index e7519f0b..13522657 100644
--- a/signac/synced_collections/buffers/file_buffered_collection.py
+++ b/signac/synced_collections/buffers/file_buffered_collection.py
@@ -364,9 +364,9 @@ class FileBufferedCollection(BufferedCollection):
         """
         if force_write is not None:
             warnings.warn(
-                DeprecationWarning(
-                    "The force_write parameter is deprecated and will be removed in "
-                    "signac 2.0. This functionality is no longer supported."
-                )
+                "The force_write parameter is deprecated and will be removed in "
+                "signac 2.0. This functionality is no longer supported.",
+                FutureWarning,
             )
+
         return cls._buffer_context(buffer_size)


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index b370f15f..fac004d0 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,19 +1,6 @@
 import uuid
-from contextlib import contextmanager
 
 import pytest
-from packaging import version
-
-import signac
-
-
-@contextmanager
-def deprecated_in_version(version_string):
-    if version.parse(version_string) <= version.parse(signac.__version__):
-        with pytest.deprecated_call():
-            yield
-    else:
-        yield
 
 
 @pytest.fixture
diff --git a/tests/test_project.py b/tests/test_project.py
index 259a296e..1263cd2d 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -20,7 +20,6 @@ from zipfile import ZipFile
 
 import pytest
 import test_h5store
-from conftest import deprecated_in_version
 from packaging import version
 from test_job import TestJobBase
 
@@ -1099,7 +1098,7 @@ class TestProject(TestProjectBase):
         assert not os.path.isdir(tmp_root_dir)
 
     def test_access_module(self):
-        with deprecated_in_version("1.5"):
+        with pytest.deprecated_call():
             self.project.create_access_module()
 
 
diff --git a/tests/test_synced_collections/test_json_collection.py b/tests/test_synced_collections/test_json_collection.py
index 4868afb7..70cabe04 100644
--- a/tests/test_synced_collections/test_json_collection.py
+++ b/tests/test_synced_collections/test_json_collection.py
@@ -52,7 +52,7 @@ class TestJSONDict(JSONCollectionTest, SyncedDictTest):
     # See issue: https://github.com/glotzerlab/signac/issues/316.
     def test_keys_non_str_valid_type(self, synced_collection, testdata):
         for key in (0, None, True):
-            with pytest.deprecated_call(match="Use of.+as key is deprecated"):
+            with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"):
                 synced_collection[key] = testdata
             assert str(key) in synced_collection
             assert synced_collection[str(key)] == testdata