Skip to content

Raise appropriate Exceptions in place of generic exceptions#2474

Merged
j9ac9k merged 4 commits intopyqtgraph:masterfrom
Nibba2018:raise_TypeError_exceptions
Oct 8, 2022
Merged

Raise appropriate Exceptions in place of generic exceptions#2474
j9ac9k merged 4 commits intopyqtgraph:masterfrom
Nibba2018:raise_TypeError_exceptions

Conversation

@Nibba2018
Copy link
Copy Markdown
Contributor

Fixes #2167
Raise TypeError in place of generic Exception for PlotDataItem.setData().
Introduced and raised a new exception named ArrayDimensionError when numpy arrays have an Invalid shape.

Other Tasks

Raise `TypeError` in place of generic `Exception` for `PlotDataItem.setData()`

Files that need updates

Confirm the following files have been either updated or there has been a determination that no update is needed.

  • README.md
  • setup.py
  • tox.ini
  • .github/workflows/main.yml and associated requirements.txt and conda environemt.yml files
  • pyproject.toml
  • binder/requirements.txt
Pre-Release Checklist

Pre Release Checklist

  • Update version info in __init__.py
  • Update CHANGELOG primarily using contents from automated changelog generation in GitHub release page
  • Have git tag in the format of pyqtgraph-
Post-Release Checklist

Steps To Complete

  • Append .dev0 to __version__ in __init__.py
  • Announce on mail list
  • Announce on Twitter

return 'Nx2array'
else:
raise Exception('array shape must be (N,) or (N,2); got %s instead' % str(obj.shape))
raise ArrayDimensionError('array shape must be (N,) or (N,2); got %s instead' % str(obj.shape))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do think it's a good idea to introduce more specific exceptions for this sort of thing, but I think for this purpose, we should probably stick with ValueError which I believe is what numpy raises when you get a dimension mismatch related exceptions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review @j9ac9k , will update it to ValueError.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 7, 2022

Hi @Nibba2018

Thanks for the PR!

I made a comment, while I think it's a good idea to have more specific exceptions, this is something that users of pyqtgraph aren't expecting, so it would create some problems for folks trying to catch this type of exception (they wouldn't know where to import it from).

I think what would be best to stick w/ numpy's convention, where I believe they raise a ValueError for this type of issue.

return 'Nx2array'
else:
raise Exception('array shape must be (N,) or (N,2); got %s instead' % str(obj.shape))
raise TypeError('array shape must be (N,) or (N,2); got %s instead' % str(obj.shape))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, this one should be a ValueError not a TypeError 👍🏻

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OMG sorry about that 🤣

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 8, 2022

diff doesn't match the comment/message, looks like it was replaced with a TypeError ;)

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 8, 2022

LGTM thanks for the contribution @Nibba2018 !

@j9ac9k j9ac9k merged commit b2e6c72 into pyqtgraph:master Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PlotDataItem.setData() raises generic Exception instead of TypeError.

2 participants