Add urls to results #146
Add urls to results #146
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 90.23% 90.63% +0.39%
==========================================
Files 5 5
Lines 635 662 +27
==========================================
+ Hits 573 600 +27
Misses 62 62 ☔ View full report in Codecov by Sentry. |
|
I'm not sure about the test fail it passes with py37 on my Mac which is running Sonoma |
|
@samaloney can you rebase? I pulled your documentation fixes into #149 |
samaloney
force-pushed
the
feat-add-urls-to-results
branch
from
97ee1ea
to
92a0841
Compare
|
oh no more conflicts, sorry! |
samaloney
force-pushed
the
feat-add-urls-to-results
branch
from
92a0841
to
3e93780
Compare
| super().__init__(*args) | ||
| self._errors = errors or list() | ||
| self._urls = urls or list() |
There was a problem hiding this comment.
Do we want some kind of assertion that urls is the same length as the list? Or maybe something even more restrictive where we have to pair them up?
parfive/downloader.py
Outdated
| results.append(filepath) | ||
| results.urls.append(requested_url) |
There was a problem hiding this comment.
Perhaps we should have an append method on results which takes both a filepath and a url?
There was a problem hiding this comment.
Seems reasonable but I think a larger refactor of the Results object will be needed soon as res = res1 + res2 works for the path but not the error and now the urls either same for other operation supported by UserList but not the results object
There was a problem hiding this comment.
So I've implemented a basic version here and created new issue around the limitations #152
samaloney
force-pushed
the
feat-add-urls-to-results
branch
from
8784d1b
to
dd83d7e
Compare
samaloney
force-pushed
the
feat-add-urls-to-results
branch
from
dd83d7e
to
60bb7d9
Compare
Trying to fix #71 specifically and as a baby step for sunpy/sunpy/issues/7140
Currently modified the return values from
_get_httpand_get_ftpand added a newurlsproperty to theResultobject. Not sure if it might be better to create a new object or named tuple to hold the url and str(path) and pass that around?