Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix TypeError with bytes docstrings #570

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tushar-deepsource
Copy link

Let me know if the fix is adequate or if it should be fixed in some other way.

Resolves #569

Pending:

  • Add unit tests and integration tests where applicable.
    If you've added an error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

@Pierre-Sassoulas
Copy link
Member

Seems like I can't approve the pipeline, do you know what's wrong with the rights @samj1912 ?

Copy link
Member

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

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

There should be an error here otherwise we're silently ignoring bytes docstrings and that is just as wrong as crashing

@tusharsadhwani
Copy link

@sigmavirus24 are you suggesting there be another lint rule?

@sigmavirus24
Copy link
Member

I'm suggesting that skipping byte string docstrings is going to be confusing for users. So we either need to try to do the right thing or emit an error code that this is a byte string and those aren't valid docstrings (because they're not if I understand the old discussion properly).

@tusharsadhwani
Copy link

@sigmavirus24 I'm in favour of simply raising a new issue for bytes then.

Trying to do the right thing has its own edge case: if we're unable to decode the bytes object, we can't do much.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash: TypeError: a bytes-like object is required, not 'str'
5 participants