Typecheck some API interfaces

[mfisher87]

I'm going to be changing a lot in the refactor to use Harmony. The typechecker is going to help find sneaky bugs along the way. This is a start, but I'd really like feedback.

My main goal here is to avoid refactoring the code more than needed and set the stage for future cleanups and refactors. E.g. I think long-term, we will want to split up the Parameters class instead of having it handle all the kinds of parameters.

I'm using pyright here for the first time, and I'm pleased with it compared to Mypy so far. The reason I switched to Pyright: python/mypy#12098

[github-actions]

👈 Launch a binder notebook on this branch for commit 5d8c38f

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit 599a40e

👈 Launch a binder notebook on this branch for commit 1299db1

👈 Launch a binder notebook on this branch for commit 4c8ec7b

👈 Launch a binder notebook on this branch for commit bf54226

👈 Launch a binder notebook on this branch for commit cb4c043

👈 Launch a binder notebook on this branch for commit cac9d1c

👈 Launch a binder notebook on this branch for commit 181099f

👈 Launch a binder notebook on this branch for commit e6e7fe2

👈 Launch a binder notebook on this branch for commit 0440fa9

👈 Launch a binder notebook on this branch for commit df78ca2

👈 Launch a binder notebook on this branch for commit 126d3fc

👈 Launch a binder notebook on this branch for commit ee70196

👈 Launch a binder notebook on this branch for commit 434c28a

👈 Launch a binder notebook on this branch for commit 2247b2c

👈 Launch a binder notebook on this branch for commit 18bd814

👈 Launch a binder notebook on this branch for commit a8996d1

👈 Launch a binder notebook on this branch for commit dfe4c09

👈 Launch a binder notebook on this branch for commit f23f5c2

👈 Launch a binder notebook on this branch for commit fddf23a

👈 Launch a binder notebook on this branch for commit e155a9b

👈 Launch a binder notebook on this branch for commit 51b3492

👈 Launch a binder notebook on this branch for commit 7f60849

👈 Launch a binder notebook on this branch for commit bb96a5f

👈 Launch a binder notebook on this branch for commit 4cb5956

👈 Launch a binder notebook on this branch for commit 7a423a8

👈 Launch a binder notebook on this branch for commit 6a4acd9

👈 Launch a binder notebook on this branch for commit e41aed0

👈 Launch a binder notebook on this branch for commit 81d60dd

[codecov]

Codecov Report

Attention: Patch coverage is 85.52632% with 11 lines in your changes missing coverage. Please review.

Project coverage is 65.66%. Comparing base (d0bc315) to head (81d60dd).
Report is 1 commits behind head on development.

Files with missing lines	Patch %	Lines
icepyx/core/granules.py	50.00%	4 Missing ⚠️
icepyx/core/query.py	69.23%	4 Missing ⚠️
icepyx/core/APIformatting.py	85.71%	0 Missing and 3 partials ⚠️

❗ There is a different number of reports uploaded between BASE (d0bc315) and HEAD (81d60dd). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (d0bc315)	HEAD (81d60dd)
	6	2

Additional details and impacted files

@@               Coverage Diff               @@
##           development     #593      +/-   ##
===============================================
- Coverage        71.60%   65.66%   -5.95%     
===============================================
  Files               37       38       +1     
  Lines             3064     3122      +58     
  Branches           596      599       +3     
===============================================
- Hits              2194     2050     -144     
- Misses             766      984     +218     
+ Partials           104       88      -16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JessicaS11]

Suggested change

	reqparams: EGISpecificParamsSearch,
	reqparams: EGISpecificParamsDownload,

[mfisher87]

is Query.reqparams always of type EGISpecificParamsDownload? Or only sometimes (this is my understanding)?

[mfisher87]

4cb5956

I think this is probably the wrong way to handle things. We're telling the typechecker "Ignore your eyes and ears, just trust us" with the cast() call. Instead, we can add a "typeguard" which is a runtime check that asserts a narrower type, e.g. a literal assert isinstance(...) or if isinstance(...): raise. I am hesitant to add runtime checks without fully grokking the code, but perhaps we do know for sure that reqparams is always EGISpecificParamsDownload at the point we're cast()ing.

[JessicaS11]

is Query.reqparams always of type EGISpecificParamsDownload? Or only sometimes (this is my understanding)?

The latter - it's only sometimes of type EGISpecificParamsDownload (when Query._reqparams._reqtype == "download"). Otherwise it's of type "search", which maps to our EGIRequiredParamsSearch.

but perhaps we do know for sure that reqparams is always EGISpecificParamsDownload at the point we're cast()ing.

I think this is true.

[JessicaS11]

Suggested change

	subsetparams,
	subsetparams: Union[EGIRequiredParamsSubset, dict[Never, Never]],

I might have the syntax wrong. In some cases we specify the type from types.py, and in others from APIformatting.py (e.g. in query.py).

[mfisher87]

dict[Never, Never] means an empty dict, and this change doesn't pass type checking. In what case would reqparams={} be passed?

[JessicaS11]

I was simply adding a type for subsetparams, since it wasn't specified. It sounds like it would need to be something more like Optional[EGIRequiredParamsSubset] so it doesn't otherwise require an empty dict.

In what case would reqparams={} be passed?

dict[Never, Never] is not being specified as a valid type to reqparams.

[mfisher87]

Woops, I meant subsetparams :)

[JessicaS11]

Approving this for merging now that we've released the last of v1 (v1.3.0). I think we've worked out most of the kinks and figure any others will be ironed out as we continue moving towards the refactor in v2. I'll be on PTO Tuesday and I know in our discussion today you said this was a critical next step for you for making progress.

[mfisher87]

Thanks, @JessicaS11 !