Skip to content

ENH: move an exception and add a prehook to check for exception place… #48088

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
Merged
merged 13 commits into from
Sep 20, 2022
Merged

ENH: move an exception and add a prehook to check for exception place… #48088

merged 13 commits into from
Sep 20, 2022

Conversation

dataxerik
Copy link
Contributor

@dataxerik dataxerik commented Aug 15, 2022 •
edited
Loading

This is a follow up from my last MR: #47901. I'm new to prehooks, so I may have went in the wrong direction for the prehook. But, I was thinking of checking the files for class that inherit Exceptions and flagging them. Let me know if another approach is preferred.

I also noticed some other exceptions while working on this.

  • InvalidVersion. I tried moving this one, but got a circular import so I'm importing it in the errors.init.py instead.
  • LossySetitemError, NoBufferPresent, and InvalidComparison are moved to errors.init.py.
  • NotThisMethod. It seem like a code generated exception rather than a custom pandas one, so I'm importing it in the errors.init.py instead.

@mroeschke mroeschke added Enhancement Error Reporting Incorrect or improved errors from pandas Code Style Code style, linting, code_checks Warnings Warnings that appear or should be added to pandas labels Aug 16, 2022
@mroeschke
Copy link
Member

Looked pretty good. Could you fix the merge conflict and see if the Ci failures are related?

@dataxerik
Copy link
Contributor Author

Looked pretty good. Could you fix the merge conflict and see if the Ci failures are related?

Sounds good. I've merged main

@mroeschke mroeschke added this to the 1.6 milestone Sep 20, 2022
@mroeschke mroeschke merged commit 744b846 into pandas-dev:main Sep 20, 2022
@mroeschke
Copy link
Member

Awesome work on this @dataxerik! This will really help make sure users are able to publicly access Warnings and Exceptions in the future!

@jorisvandenbossche
Copy link
Member

(just getting here from the notification that the issue was closed, so sorry for the late comment)

I think that InvalidComparison is only used internally to raise certain cases which is then catched internally again to do something else or raise a public error. Assuming that's the case, we don't need to expose this exception class publicly?

(the same might be true for LossySetitemError, didn't check in detail, cc @jbrockmendel)

@mroeschke
Copy link
Member

That's a fair point.

Maybe we can clearly document the exceptions or warnings that are supposed to be internal? My concern is that overtime the "internal" exceptions or warnings may unintentionally become publicly thrown, and at least users could still catch them through pandas.errors

@jbrockmendel
Copy link
Member

I think that InvalidComparison is only used internally to raise certain cases which is then catched internally again to do something else or raise a public error. Assuming that's the case, we don't need to expose this exception class publicly?

correct. It should always be caught and re-raised. Could clarify that in the docstring.

Ditto for LossySetitemError.

@mroeschke mroeschke mentioned this pull request Sep 21, 2022
Closed
phofl pushed a commit to phofl/pandas that referenced this pull request Sep 22, 2022
@dataxerik @phofl
ENH: move an exception and add a prehook to check for exception place… (
b9ec72b 
pandas-dev#48088)

* ENH: move an exception and add a prehook to check for exception placement

* ENH: fix import

* ENH: revert moving error

* ENH: add docstring and fix import for test

* ENH: re-design approach based on feedback

* ENH: update whatsnew rst

* ENH: apply feedback changes

* ENH: refactor to remove exception_warning_list and ignore _version.py

* ENH: remove NotThisMethod from tests and all
mroeschke added a commit that referenced this pull request Sep 26, 2022
@phofl @mroeschke
@rgommers @datapythonista @lukemanley @gandhis1 @twoertwein @yuanx749 @paradox-lab @pnacht @dataxerik @jbrockmendel @PabloRuizCuevas @tmoschou @lithomas1 @rhshadrach
BUG: Series.getitem not falling back to positional for bool index (#4…
2a6c6a2 
…8662)

* BUG: Series.getitem not falling back to positional for bool index

* Update pandas/tests/series/indexing/test_getitem.py

Co-authored-by: Matthew Roeschke <[email protected]>

* Fix build warning for use of `strdup` in ultrajson (#48369)

* WEB: Update versions json to fix version switcher in the docs (#48655)

* PERF: join/merge on subset of MultiIndex (#48611)

* DOC: Update documentation for date_range(), bdate_range(), and interval_range() to include timedelta as a possible data type for the freq parameter (#48631)

* Update documentation for date_range(), bdate_range(), and interval_range() to include timedelta as a possible data type for the freq parameter

* Add test case for date_range construction using datetime.timedelta

* TYP: tighten Axis (#48612)

* TYP: tighten Axis

* allow 'rows'

* BUG: Fix metadata propagation in df.corr and df.cov, GH28283 (#48616)

* Add finalize to df.corr and df.cov

* Clean

* TST: add test case for PeriodIndex in HDFStore(GH7796) (#48618)

* TST: add test case for PeriodIndex in HDFStore

* TST: add test case for PeriodIndex in HDFStore

* use pytest.mark.parameterize instead

* Add OpenSSF Scorecards GitHub Action (#48570)

* Create scorecards.yml

* Update scorecards.yml

* Add OpenSSF Scorecards badge to README.md

* Trim whitespace in scorecards.yml

* Skip scorecards.yml on forks

* Fix whitespace

* Pin scorecards.yml dependencies to major versions

* ENH: move an exception and add a prehook to check for exception place… (#48088)

* ENH: move an exception and add a prehook to check for exception placement

* ENH: fix import

* ENH: revert moving error

* ENH: add docstring and fix import for test

* ENH: re-design approach based on feedback

* ENH: update whatsnew rst

* ENH: apply feedback changes

* ENH: refactor to remove exception_warning_list and ignore _version.py

* ENH: remove NotThisMethod from tests and all

* REGR: TextIOWrapper raising an error in read_csv (#48651)

* REGR: TextIOWrapper raising an error in read_csv

* pyupgrade

* do not try to seek on unseekable buffers

* unseekable buffer might also have read ahead

* safer alternative: do not mess with internal/private(?) buffer of TextIOWrapper (effectively applies the shortcut only to files pandas opens)

* Fix scorecard.yml workflow (#48668)

* Set scorecard-action to v2.0.3

scorecard-action does not have a major version tag.

Temporarily disabling github.repository check to ensure action now works.

* Enable github.repository check

* BUG: DatetimeIndex ignoring explicit tz=None (#48659)

* BUG: DatetimeIndex ignoring explicit tz=None

* GH ref

* Corrected pd.merge indicator type hint (#48677)

* Corrected pd.merge indicator type hint

https://pandas.pydata.org/docs/reference/api/pandas.merge.html
It should be "str | bool" instead of just string

* Update merge.py

fixed type hint in merge.py

* Update merge.py

Update indicator type hint in _MergeOperation

* Update merge.py

Added type hint _MergeOperation init

* DOC: Document default value for options.display.max_cols when not running in terminal (#48672)

DOC: Document default value for options.display.max_cols

display.max_cols has a default value of 20 when not running in a terminal
such as Jupyter Notebook

* ENH: DTA/TDA add datetimelike scalar with mismatched reso (#48669)

* ENH: DTA/TDA add datetimelike scalar with mismatched reso

* mypy fixup

* REF: support reso in remaining tslibs helpers (#48661)

* REF: support reso in remaining tslibs helpers

* update setup.py

* PERF: Avoid fragmentation of DataFrame in read_sas (#48603)

* PERF: Avoid fragmentation of DataFrame in read_sas

* Add whatsnew

* Add warning

* DOC: Add deprecation infos to deprecated functions (#48599)

* DOC: Add deprecation infos to deprecated functions

* Add sections

* Fix

* BLD: Build wheels using cibuildwheel (#48283)

* BLD: Build wheels using cibuildwheel

* update from code review

Co-Authored-By: Matthew Roeschke <[email protected]>

* fix 3.11 version

* changes from code review

* Update test_wheels.py

* sync run time with pandas-wheels

Co-authored-by: Matthew Roeschke <[email protected]>

* REGR: Performance decrease in factorize (#48620)

* TYP: type all arguments with str default values (#48508)

* TYP: type all arguments with str default values

* na_rep: back to str

* na(t)_rep is always a string

* add float for some functions

* and the same for the few float default arguments

* define a few more literal constants

* avoid itertools.cycle mypy error

* revert mistake

* TST: Catch more pyarrow PerformanceWarnings (#48699)

* REGR: to_hdf raising AssertionError with boolean index (#48696)

* REGR: to_hdf raising AssertionError with boolean index

* Add gh ref

* REGR: Regression in DataFrame.loc when setting df with all True indexer (#48711)

* BUG: pivot_table raising for nullable dtype and margins (#48714)

* TST: Address MPL 3.6 deprecation warnings (#48695)

* TST: Address MPL 3.6 deprecation warnings

* Address min build

* missing ()

Co-authored-by: Matthew Roeschke <[email protected]>
Co-authored-by: Ralf Gommers <[email protected]>
Co-authored-by: Marc Garcia <[email protected]>
Co-authored-by: Luke Manley <[email protected]>
Co-authored-by: Siddhartha Gandhi <[email protected]>
Co-authored-by: Torsten Wörtwein <[email protected]>
Co-authored-by: Xiao Yuan <[email protected]>
Co-authored-by: paradox-lab <[email protected]>
Co-authored-by: Pedro Nacht <[email protected]>
Co-authored-by: dataxerik <[email protected]>
Co-authored-by: jbrockmendel <[email protected]>
Co-authored-by: Pablo <[email protected]>
Co-authored-by: tmoschou <[email protected]>
Co-authored-by: Thomas Li <[email protected]>
Co-authored-by: Richard Shadrach <[email protected]>
@mroeschke mroeschke modified the milestones: 1.6, 2.0 Oct 13, 2022
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
@dataxerik @noatamir
ENH: move an exception and add a prehook to check for exception place… (
242b563 
pandas-dev#48088)

* ENH: move an exception and add a prehook to check for exception placement

* ENH: fix import

* ENH: revert moving error

* ENH: add docstring and fix import for test

* ENH: re-design approach based on feedback

* ENH: update whatsnew rst

* ENH: apply feedback changes

* ENH: refactor to remove exception_warning_list and ignore _version.py

* ENH: remove NotThisMethod from tests and all
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
@phofl @mroeschke
@rgommers @datapythonista @lukemanley @gandhis1 @twoertwein @yuanx749 @paradox-lab @pnacht @dataxerik @jbrockmendel @PabloRuizCuevas @tmoschou @lithomas1 @rhshadrach @noatamir
BUG: Series.getitem not falling back to positional for bool index (pa…
127beb0 
…ndas-dev#48662)

* BUG: Series.getitem not falling back to positional for bool index

* Update pandas/tests/series/indexing/test_getitem.py

Co-authored-by: Matthew Roeschke <[email protected]>

* Fix build warning for use of `strdup` in ultrajson (pandas-dev#48369)

* WEB: Update versions json to fix version switcher in the docs (pandas-dev#48655)

* PERF: join/merge on subset of MultiIndex (pandas-dev#48611)

* DOC: Update documentation for date_range(), bdate_range(), and interval_range() to include timedelta as a possible data type for the freq parameter (pandas-dev#48631)

* Update documentation for date_range(), bdate_range(), and interval_range() to include timedelta as a possible data type for the freq parameter

* Add test case for date_range construction using datetime.timedelta

* TYP: tighten Axis (pandas-dev#48612)

* TYP: tighten Axis

* allow 'rows'

* BUG: Fix metadata propagation in df.corr and df.cov, GH28283 (pandas-dev#48616)

* Add finalize to df.corr and df.cov

* Clean

* TST: add test case for PeriodIndex in HDFStore(GH7796) (pandas-dev#48618)

* TST: add test case for PeriodIndex in HDFStore

* TST: add test case for PeriodIndex in HDFStore

* use pytest.mark.parameterize instead

* Add OpenSSF Scorecards GitHub Action (pandas-dev#48570)

* Create scorecards.yml

* Update scorecards.yml

* Add OpenSSF Scorecards badge to README.md

* Trim whitespace in scorecards.yml

* Skip scorecards.yml on forks

* Fix whitespace

* Pin scorecards.yml dependencies to major versions

* ENH: move an exception and add a prehook to check for exception place… (pandas-dev#48088)

* ENH: move an exception and add a prehook to check for exception placement

* ENH: fix import

* ENH: revert moving error

* ENH: add docstring and fix import for test

* ENH: re-design approach based on feedback

* ENH: update whatsnew rst

* ENH: apply feedback changes

* ENH: refactor to remove exception_warning_list and ignore _version.py

* ENH: remove NotThisMethod from tests and all

* REGR: TextIOWrapper raising an error in read_csv (pandas-dev#48651)

* REGR: TextIOWrapper raising an error in read_csv

* pyupgrade

* do not try to seek on unseekable buffers

* unseekable buffer might also have read ahead

* safer alternative: do not mess with internal/private(?) buffer of TextIOWrapper (effectively applies the shortcut only to files pandas opens)

* Fix scorecard.yml workflow (pandas-dev#48668)

* Set scorecard-action to v2.0.3

scorecard-action does not have a major version tag.

Temporarily disabling github.repository check to ensure action now works.

* Enable github.repository check

* BUG: DatetimeIndex ignoring explicit tz=None (pandas-dev#48659)

* BUG: DatetimeIndex ignoring explicit tz=None

* GH ref

* Corrected pd.merge indicator type hint (pandas-dev#48677)

* Corrected pd.merge indicator type hint

https://pandas.pydata.org/docs/reference/api/pandas.merge.html
It should be "str | bool" instead of just string

* Update merge.py

fixed type hint in merge.py

* Update merge.py

Update indicator type hint in _MergeOperation

* Update merge.py

Added type hint _MergeOperation init

* DOC: Document default value for options.display.max_cols when not running in terminal (pandas-dev#48672)

DOC: Document default value for options.display.max_cols

display.max_cols has a default value of 20 when not running in a terminal
such as Jupyter Notebook

* ENH: DTA/TDA add datetimelike scalar with mismatched reso (pandas-dev#48669)

* ENH: DTA/TDA add datetimelike scalar with mismatched reso

* mypy fixup

* REF: support reso in remaining tslibs helpers (pandas-dev#48661)

* REF: support reso in remaining tslibs helpers

* update setup.py

* PERF: Avoid fragmentation of DataFrame in read_sas (pandas-dev#48603)

* PERF: Avoid fragmentation of DataFrame in read_sas

* Add whatsnew

* Add warning

* DOC: Add deprecation infos to deprecated functions (pandas-dev#48599)

* DOC: Add deprecation infos to deprecated functions

* Add sections

* Fix

* BLD: Build wheels using cibuildwheel (pandas-dev#48283)

* BLD: Build wheels using cibuildwheel

* update from code review

Co-Authored-By: Matthew Roeschke <[email protected]>

* fix 3.11 version

* changes from code review

* Update test_wheels.py

* sync run time with pandas-wheels

Co-authored-by: Matthew Roeschke <[email protected]>

* REGR: Performance decrease in factorize (pandas-dev#48620)

* TYP: type all arguments with str default values (pandas-dev#48508)

* TYP: type all arguments with str default values

* na_rep: back to str

* na(t)_rep is always a string

* add float for some functions

* and the same for the few float default arguments

* define a few more literal constants

* avoid itertools.cycle mypy error

* revert mistake

* TST: Catch more pyarrow PerformanceWarnings (pandas-dev#48699)

* REGR: to_hdf raising AssertionError with boolean index (pandas-dev#48696)

* REGR: to_hdf raising AssertionError with boolean index

* Add gh ref

* REGR: Regression in DataFrame.loc when setting df with all True indexer (pandas-dev#48711)

* BUG: pivot_table raising for nullable dtype and margins (pandas-dev#48714)

* TST: Address MPL 3.6 deprecation warnings (pandas-dev#48695)

* TST: Address MPL 3.6 deprecation warnings

* Address min build

* missing ()

Co-authored-by: Matthew Roeschke <[email protected]>
Co-authored-by: Ralf Gommers <[email protected]>
Co-authored-by: Marc Garcia <[email protected]>
Co-authored-by: Luke Manley <[email protected]>
Co-authored-by: Siddhartha Gandhi <[email protected]>
Co-authored-by: Torsten Wörtwein <[email protected]>
Co-authored-by: Xiao Yuan <[email protected]>
Co-authored-by: paradox-lab <[email protected]>
Co-authored-by: Pedro Nacht <[email protected]>
Co-authored-by: dataxerik <[email protected]>
Co-authored-by: jbrockmendel <[email protected]>
Co-authored-by: Pablo <[email protected]>
Co-authored-by: tmoschou <[email protected]>
Co-authored-by: Thomas Li <[email protected]>
Co-authored-by: Richard Shadrach <[email protected]>
@DeaMariaLeon DeaMariaLeon mentioned this pull request Jul 21, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Code Style Code style, linting, code_checks Enhancement Error Reporting Incorrect or improved errors from pandas Warnings Warnings that appear or should be added to pandas
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

API: many custom errors / warnings are not exposed in pandas.errors
4 participants
@dataxerik @mroeschke @jorisvandenbossche @jbrockmendel