Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use FutureWarning instead of DeprecationWarning in project.py #691

Merged
merged 2 commits into from
Feb 28, 2022

Conversation

Onkar627
Copy link
Contributor

@Onkar627 Onkar627 commented Feb 26, 2022

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 change1

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

Checklist:

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.

@Onkar627 Onkar627 requested review from a team as code owners February 26, 2022 05:56
@Onkar627 Onkar627 requested review from tcmoore3 and klywang and removed request for a team February 26, 2022 05:56
@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #691 (f92adb4) into master (c291e1e) will increase coverage by 0.01%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #691      +/-   ##
==========================================
+ 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 Coverage Δ
signac/contrib/project.py 85.03% <57.14%> (ø)
signac/__main__.py 71.89% <0.00%> (+0.11%) ⬆️

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 c291e1e...f92adb4. Read the comment docs.

Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Onkar627, thank you for the pull request! It looks like the changes in this PR are good. I have a few requests for this PR before merging:

  1. An important part of the pull request process is to include descriptions of what was changed and why. Descriptions and references to related issues helps reviewers and PR authors communicate effectively. Would you be able to write some description/motivation information in the top section of the PR description? Please include a link to the issue Use FutureWarning to report deprecations #687. Look at past pull requests for examples of what kinds of information to write.
  2. Please rename this PR to "Use FutureWarning instead of DeprecationWarning in project.py" so that the intention of the pull request is clear.
  3. Look over the PR checklist and mark the appropriate boxes. For example, this PR would probably be considered a "New feature." Additionally, you need to read over and check the boxes below that to acknowledge that you have read the contributing guidelines, agree to the contributor agreement, have added yourself to the contributor list, etc.

@Onkar627 Onkar627 changed the title Update project.py Use FutureWarning instead of DeprecationWarning in project.py Feb 28, 2022
@Onkar627
Copy link
Contributor Author

I have updated the PR @bdice ,please have a look and just ping if any changes are required :)

@bdice
Copy link
Member

bdice commented Feb 28, 2022

@Onkar627 Thank you! If you would like, please consider adding yourself to the list of contributors.

I'll add a changelog entry, and then merge this PR after you add yourself to the contributor list (you may decline if you'd rather not add yourself).

We will use the same changelog entry for #692, and just add the number of that PR in the same line.

@bdice bdice added the enhancement New feature or request label Feb 28, 2022
@bdice bdice added this to the v1.8.0 milestone Feb 28, 2022
@Onkar627
Copy link
Contributor Author

@Onkar627 Thank you! If you would like, please consider adding yourself to the list of contributors.

I'll add a changelog entry, and then merge this PR after you add yourself to the contributor list (you may decline if you'd rather not add yourself).

We will use the same changelog entry for #692, and just add the number of that PR in the same line.

Surely,I will love to add.

@bdice bdice merged commit 1285881 into glotzerlab:master Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants