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.

Sign up for GitHub

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

Jump to bottom

Prevent globally overriding warnings.simplefilter #293

Closed
wants to merge 3 commits into from
Closed

Prevent globally overriding warnings.simplefilter #293

wants to merge 3 commits into from

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented Oct 23, 2020 •
edited
Loading

Modify warnings.simplefilter only when needed (raising
DeprecationWarnings) instead of doing it globally affecting every
other Python library.

Fixes #290

Reminders:

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst and the base __init__.py file for the package.
  • Write detailed docstrings for all functions/classes/methods. It often helps to design better code if you write the docstrings first.
  • If adding new functionality, add an example to the docstring, gallery, and/or tutorials.
  • Add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file (if you haven't already) in case you'd like to be listed as an author on the Zenodo archive of the next release.

@santisoler
Prevent globally overriding warnings simplefilter
fc49f98 
Modify warnings.simplefilter only when needed (raising
DeprecationWarnings) instead of doing it globally what affects every
other Python library.
@santisoler
Copy link
Member Author

@freeman-lab would you review this PR and check if it actually solves the issue.
It works for me at least, but would be nice to hear about your experience.

@santisoler santisoler changed the title Prevent globally overriding warnings simplefilter Prevent globally overriding warnings.simplefilter Oct 23, 2020
@santisoler santisoler requested a review from leouieda October 23, 2020 19:31
leouieda
leouieda reviewed Oct 24, 2020
View reviewed changes
Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Looks good to me. I’ll check locally as well when I get a chance.

@leouieda
Copy link
Member

It seems to work fine for me on my machine.

@leouieda
Copy link
Member

leouieda commented Dec 8, 2020

Apparently there is a good reason for DeprecationWarnings to not show by default: they are meant for developers not end users. We should actually be issuing FutureWarnings instead (which i think do get show by default). So we could actually remove the filter altogether and change our warning type instead. What do you think?

@santisoler santisoler mentioned this pull request Feb 18, 2021
Merged
6 tasks
@santisoler
Copy link
Member Author

Apparently there is a good reason for DeprecationWarnings to not show by default: they are meant for developers not end users. We should actually be issuing FutureWarnings instead (which i think do get show by default). So we could actually remove the filter altogether and change our warning type instead. What do you think?

I agree! I've opened #305 to replace every DeprecationWarnings for FutureWarnings and remove the filter.
I leave this link for future reference about this topic: https://stackoverflow.com/questions/55377831/difference-between-deprecationwarning-pendingdeprecationwarning-and-futurewarni

I'm closing this PR on behalf of #305. Thanks @leouieda for the insight and @freeman-lab for the issue.

@santisoler santisoler closed this Feb 18, 2021
leouieda pushed a commit that referenced this pull request Mar 16, 2021
@santisoler
Replace DeprecationWarning for FutureWarning (#305)
4cabc4a 
`DeprecationWarning`s are intended for developers, while the warnings 
we want to raise are to let users know that a feature will be deprecated 
is the `FutureWarning`. Remove the override of `warning.simplefilter`.
Following the comments in #293

Fixes #290
jessepisel added a commit to jessepisel/verde that referenced this pull request Apr 13, 2021
@jessepisel @santisoler
@fatiando-bot @leouieda
update fork (#4)
253821d 
* Fix typo on "coordinates" (fatiando#306)

Fix a common typo on Verde: replace "coordiantes" for "coordinates".

* Get INSTALL_REQUIRES from requirements.txt (fatiando#312)

On setup.py, build the INSTALL_REQUIRES variable by reading the requirements.txt file.

* Include requirements.txt in MANIFEST.in (fatiando#313)

* Exclude Dask 2021.03.0 as a dependency (fatiando#311)

Dask 2021.03.0 was causing the tests to fail under Python 3.8 on any OS.
The problem seems to be originated by dask.distributed.
By excluding this version, we avoid the problem in the future.

* Replace Travis and Azure for GitHub Actions (fatiando#309)

Remove Azure and Travis configuration files.
Add Github Actions for continuous integration and deployment.
Related to fatiando/maintenance#1

* Replace versioneer for setuptools-scm (fatiando#307)

Replace `versioneer` with `setuptools_scm` for getting Semver version of
Verde. `setuptools_scm` doesn't require to store additional files to work,
it can be installed and used through `setup.py`. Remove all `versioneer`
related files and mentions. Add `setuptools_scm` to `requirements.txt` and
`environment.yml`. Replace `setup.cfg` for `.flake8`. 
Related to fatiando/maintenance#4

* Replace DeprecationWarning for FutureWarning (fatiando#305)

`DeprecationWarning`s are intended for developers, while the warnings 
we want to raise are to let users know that a feature will be deprecated 
is the `FutureWarning`. Remove the override of `warning.simplefilter`.
Following the comments in fatiando#293

Fixes fatiando#290

* Add license notice to every source file (fatiando#308)

Add a license notice to every source file and a script for automatically
check if the license is present on every file and add it if missing.

* Update files from fatiando/contributing (fatiando#314)

Changes have been made in https://github.com/fatiando/contributing to:
MAINTENANCE.md
Update the copies in this repository to match.
See fatiando/community@b78e925

* Add .eggs to .gitignore (fatiando#315)

Ignore the .eggs directory that is created after `make install`

* Allow make_xarray_grid to take horizontal coordinates as 1d arrays (fatiando#300)

Now make_xarray_grid could take 1d arrays for horizontal coordinates, such as
how xr.DataArray stores the single dimension coordinates. Add a new
non-public meshgrid_to_1d function to convert 2d meshgrids (like the ones
generated by vd.grid_coordinates) to 1d arrays containing a single coordinate
of the corresponding axis. This function checks if all elements inside the 2d
array are equal along the right axis, i.e. a vaild meshgrid.

* Add changelog entry for v1.6.0 (fatiando#316)

* Fix wrong version numbers for PyPI releases (fatiando#317)

For test releases to TestPyPI we have to edit the setuptools_scm
configuration to produce valid version numbers. But it turns out that
this breaks the number for actual releases. To fix it, only edit the
configuration on non-release builds.

* Allow make_xarray_grid to get data as None (fatiando#318)

Used to generate a `xr.Dataset` without any `data_vars` array, 
containing only the coordinates. Will be useful in some cases 
to define an `xr.Dataset` with coordinates only and add the 
`data_vars` afterwards.

* Changelog entry for v1.6.1 (fatiando#320)

Minor release to include a small fix to make_xarray_grid

* Update files from fatiando/contributing (fatiando#321)

Changes have been made in https://github.com/fatiando/contributing to:
MAINTENANCE.md
Update the copies in this repository to match.
See fatiando/community@26724ee

* Extend support for Python 3.9 (fatiando#323)

Add Python 3.9 to the CI and setup.py flags

Co-authored-by: Santiago Soler <santiago.r.soler@gmail.com>
Co-authored-by: Fatiando a Terra Bot <50936856+fatiando-bot@users.noreply.github.com>
Co-authored-by: Leonardo Uieda <leouieda@gmail.com>
@leouieda leouieda deleted the deprecation-warn branch June 30, 2022 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leouieda leouieda leouieda left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change the scope of warning settings to avoid import time side effects
2 participants
@santisoler @leouieda