Page MenuHomeSoftware Heritage

Add type annotations to swh.core.api
AbandonedPublic

Authored by legau on Feb 7 2020, 11:09 AM.

Details

Summary

Annotation of methods for files swh/core/negotation.py and swh/core/serializers.py

Diff Detail

Repository
rDCORE Foundations and core functionalities
Branch
type-annotation
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10524
Build 15692: tox-on-jenkinsJenkins
Build 15691: arc lint + arc unit

Event Timeline

legau created this revision.Feb 7 2020, 11:09 AM
vlorentz requested changes to this revision.Feb 7 2020, 11:17 AM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/core/api/negotiation.py
58

bytes

66

bytes

140

missing type of err

swh/core/api/serializers.py
155

bytes

180

For consistency, either use Any for this return type, or use Union[int, dict, str, bytes] in all other places when dealing with deserialized data

This revision now requires changes to proceed.Feb 7 2020, 11:17 AM
legau updated this revision to Diff 9430.Feb 7 2020, 11:28 AM
legau marked 5 inline comments as done.
  • Fixed consistency problems
vlorentz requested changes to this revision.Feb 7 2020, 3:20 PM
vlorentz added inline comments.
swh/core/api/negotiation.py
140

err is optional

This revision now requires changes to proceed.Feb 7 2020, 3:20 PM
legau added inline comments.Feb 7 2020, 3:27 PM
swh/core/api/negotiation.py
140

If this err should be annoted Optional, should all other parameters with default values be Optional as well ?

legau marked an inline comment as not done.Feb 7 2020, 3:29 PM
vlorentz added inline comments.Feb 7 2020, 4:46 PM
swh/core/api/negotiation.py
140
legau added inline comments.Feb 7 2020, 5:15 PM
swh/core/api/negotiation.py
140

Yes I meant for the other parameters where None is the default parameters. For example

def get_formatter(self, format: str = None,
                  mimetype: str = None) -> Formatter:

line 102.

vlorentz added inline comments.Feb 10 2020, 12:10 PM
swh/core/api/negotiation.py
140

then yes

legau updated this revision to Diff 9462.Feb 10 2020, 4:05 PM
  • Optional params are now Optional
legau marked 3 inline comments as done.Feb 10 2020, 4:09 PM
vlorentz requested changes to this revision.Feb 10 2020, 4:49 PM

One last comment, and it should be good.

Could you also squash your commits?

swh/core/api/serializers.py
180

Sorry I missed this earlier, but data should have type bytes.

This revision now requires changes to proceed.Feb 10 2020, 4:49 PM
legau marked an inline comment as done.Feb 10 2020, 5:07 PM
legau abandoned this revision.Feb 11 2020, 2:14 PM