Use FutureWarning instead of DeprecationWarning.

[Onkar627]

Description

Fixes issues as described by @vyasr in Issue #687 as we tend to change the DeprecationWarning to FutureWarning instead.

Motivation and Context

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

• Documentation update
• Bug fix
• New feature
• Breaking change¹

¹The change breaks (or has the potential to break) existing functionality.

Checklist:

• I am familiar with the Contributing Guidelines.
• I agree with the terms of the Contributor Agreement.
• My name is on the list of contributors.
• My code follows the code style guideline of this project.
• 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 on signac-docs and linked it here.
• I have updated the changelog and added all related issue and pull request numbers for future reference (if applicable). See example below.

[codecov]

Codecov Report

Merging #692 (234c34e) into master (a5937cb) will not change coverage.
The diff coverage is n/a.

❗ Current head 234c34e differs from pull request most recent head d51ec0c. Consider uploading reports for the commit d51ec0c to get more accurate results

@@           Coverage Diff           @@
##           master     #692   +/-   ##
=======================================
  Coverage   78.54%   78.54%           
=======================================
  Files          65       65           
  Lines        7141     7141           
  Branches     1565     1565           
=======================================
  Hits         5609     5609           
  Misses       1228     1228           
  Partials      304      304           

Impacted Files	Coverage Δ
signac/contrib/indexing.py	70.95% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5937cb...d51ec0c. Read the comment docs.

[bdice]

Hi @Onkar627, thank you for the pull request! It looks like the changes in this PR are good, but incomplete. I have a few requests for this PR before merging - please review my comment on #691 for some basic steps needed for all pull requests (description, title, checklist),

Additionally, this PR should update a few other DeprecationWarnings that were missed. These can be found in signac/synced_collections/backends/collection_json.py and signac/synced_collections/buffers/file_buffered_collection.py. I have included direct links below.

signac/signac/synced_collections/backends/collection_json.py

Line 52 in c291e1e

DeprecationWarning,

signac/signac/synced_collections/backends/collection_json.py

Line 149 in c291e1e

DeprecationWarning,

signac/signac/synced_collections/buffers/file_buffered_collection.py

Line 367 in c291e1e

DeprecationWarning(

Finally, we may need to update the warning filters in tests/test_project.py and tests/test_job.py. These may need to hide FutureWarning instead of DeprecationWarning. You can check the pytest output to see what warnings are raised.

signac/tests/test_project.py

Line 2597 in c291e1e

warnings.filterwarnings("ignore", category=DeprecationWarning, module="signac")

signac/tests/test_job.py

Line 81 in c291e1e

warnings.filterwarnings("ignore", category=DeprecationWarning, module="signac")

[bdice]

This file is vendored (copied from an upstream project) so we usually avoid changing it. We have control over how this object is created in the library and it's not a part of user-facing code, so I suggest reverting this change.

Suggested change

	FutureWarning,
	DeprecationWarning,

[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.

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:

- with pytest.deprecated_call(match="Use of.+as key is deprecated"):
+ with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"):

[bdice]

Hmm, something was wrong here in the old code. Can you switch this to call warnings.warn(message, FutureWarning) instead of warnings.warn(FutureWarning(message))? The function isn't being used correctly.

https://docs.python.org/3/library/warnings.html#warnings.warn

[Onkar627]

Yeah, sure.

[mikemhenry]

Tests are failing,

    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"):
>               synced_collection[key] = testdata
E               Failed: DID NOT WARN. No warnings of type (<class 'DeprecationWarning'>, <class 'PendingDeprecationWarning'>) were emitted. The list of emitted warnings is: [FutureWarning('Use of int as key is deprecated and will be removed in version 2.0.')].

So they need to be updated

[Onkar627]

@Onkar627 There are some tests failing. See logs here for linux-python-38.

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:

- 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:

def deprecated_in_version(version_string):
   if version.parse(version_string) <= version.parse(signac.__version__):
- with pytest.deprecated_call():
+ with pytest.warns(FutureWarning):

@bdice

[bdice]

Thanks @Onkar627! I pushed a commit to address your question about conftest.py, that was a good observation. That function deprecated_in_version was outdated and only used once, so I removed it. I'm going to merge this PR so that I can do some related follow-up work.