search

AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.45k stars 287 forks source link

Added support to filepath_from_url for UNC paths and tests for UNC and posix paths #1674

Closed douglascomet closed 6 months ago

douglascomet commented 11 months ago

This PR is a follow up to https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1664 to add support to filepath_from_url and tests for UNC paths.

douglascomet commented 11 months ago

Just occurred to me, should filepath_from_url raise or do nothing if a urlstr does not have the file:// prefix? Same question if a url with a different scheme is provided?

codecov-commenter commented 11 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (d22935e) 79.84% compared to head (1ad7b70) 79.84%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1674 +/- ## ======================================= Coverage 79.84% 79.84% ======================================= Files 197 197 Lines 21796 21820 +24 Branches 4358 4363 +5 ======================================= + Hits 17403 17423 +20 - Misses 2232 2234 +2 - Partials 2161 2163 +2 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `79.84% <87.09%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [tests/test\_url\_conversions.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF91cmxfY29udmVyc2lvbnMucHk=) | `95.34% <100.00%> (+2.49%)` | :arrow_up: | | [src/py-opentimelineio/opentimelineio/url\_utils.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL3B5LW9wZW50aW1lbGluZWlvL29wZW50aW1lbGluZWlvL3VybF91dGlscy5weQ==) | `85.18% <63.63%> (-14.82%)` | :arrow_down: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [d22935e...1ad7b70](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1674?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
meshula commented 7 months ago

@douglascomet Does your latest commit resolve either of the conversations?

douglascomet commented 7 months ago

@douglascomet Does your latest commit resolve either of the conversations?

@meshula Yes, it will. Apologies for the delay with pushing this PR forward.

meshula commented 7 months ago

@douglascomet No need to apologize, I was just checking status, no urgency.

reinecke commented 6 months ago

@douglascomet in response to:

Just occurred to me, should filepath_from_url raise or do nothing if a urlstr does not have the file:// prefix? Same question if a url with a different scheme is provided?

I would say it should not raise - the implication is that that is a relative URL. That does, however, imply that filepath_from_url should maybe accepts something like a base_path with is the filepath that paths can be relative to (usually this would be the path of parent dir the loaded otio file is in).

But I think this is out of scope for this PR.