search

datalad / datalad-next

DataLad extension for new functionality and improved user experience
https://datalad.org
Other
7 stars 8 forks source link

Relax type assertion from PurePosixPath to PosixPath in RIA patches #696

Closed mih closed 4 months ago

mih commented 4 months ago

The patched code is new patch-code, introduced in https://github.com/datalad/datalad-next/pull/669

The assertions were added to make sure that no platform paths are passed around. The Debian maintainer observed non-pure paths coming in. This may need an investigation, but this changeset maintains the spirit of the assertions while relaxing them enough to let the tests pass in a Debian build environment.

Ping @christian-monch WDYT?

This patch is part of Debian's 1.4.0-1

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.43%. Comparing base (e8647c4) to head (7503479). Report is 141 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #696 +/- ## ========================================== + Coverage 92.42% 92.43% +0.01% ========================================== Files 193 193 Lines 14121 14121 Branches 2128 2128 ========================================== + Hits 13051 13053 +2 + Misses 808 806 -2 Partials 262 262 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mih commented 4 months ago

Likely also the cause for the breakage observed in https://github.com/datalad-handbook/book/pull/1223

mih commented 4 months ago

@christian-monch is not immediately available for a review at this point. Given the relatively large impact, I am going ahead and merge this change. We may need to revisit the decisions made here at a later point in time.

christian-monch commented 4 months ago

Thanks @mih. It's good to have the tests pass.

I think we have to revisit the approach though. I looked into issue #705. While this patch turns the tests green, it masks an error in the datalad-next extension tests. The error was due to the fact that extensions were not loaded early enough (see this comment for details). I opened PR datalad #7605, that basically sets the environment variable DATALAD_EXTENSION_TESTS=next when running the datalad-next extension tests. Locally this turns the datalad-next extension tests in datalad-core green with datalad-next version 1.4.0. So the test should still run if the commits in this PR would be reverted