@jsolbrig noticed that some portions of the GeoIPS codebase implement functionality that will only work for packages installed in editable mode. This was found in PR #444, where geoips test linting lints packages at their top level. This is done via importlib.resources.files(<pkg_name>) / "../.", so we could lint everything included in the package. While this works for packages installed in editable mode, it will not work for packages installed without the -e flag. This is because these packages will be referenced in site-packages, and by performing the ../. directory maneuver, we'll actually be linting everything included in site-packages.
Background and Motivation
This stems from a conversation that @jsolbrig and I had when testing the CLI. This fix is not needed immediately, but should be done ASAP to prevent bugs slipping into the code when packages are installed in non-editable mode. Off the top of my head, GeoipsListScripts, GeoipsTestScript, and the initial implementation of GeoipsRun will fail if installed in non-editable mode. This is because test scripts will not be available as they are in editable mode. We should search the codebase for any reference of resources.files(<pkg_name>) / "../" and rethink whether or not this functionality is needed, as it won't work for a non-editable installed package.
Code to demonstrate issue
In PR #444 the following commands will not work if installed in non-editable mode
GeoipsListUnitTests
GeoipsListScripts
GeoipsRun (this will not matter after #465 is merged)
GeoipsTestScript
GeoipsTestLinting
There might be other calls that perform this in other parts of the code base
Checklist for Completion
[ ] Find all locations in the codebase which use resources.files(<pkg_name>) / "../"
[ ] Reconsider whether or not we want to support the related functionality at all
[ ] If we do, add checks to see whether or not the package is installed in editable mode
Requested Update
Description
@jsolbrig noticed that some portions of the GeoIPS codebase implement functionality that will only work for packages installed in editable mode. This was found in PR #444, where
geoips test lintinglints packages at their top level. This is done viaimportlib.resources.files(<pkg_name>) / "../.", so we could lint everything included in the package. While this works for packages installed in editable mode, it will not work for packages installed without the-eflag. This is because these packages will be referenced insite-packages, and by performing the../.directory maneuver, we'll actually be linting everything included insite-packages.Background and Motivation
This stems from a conversation that @jsolbrig and I had when testing the CLI. This fix is not needed immediately, but should be done ASAP to prevent bugs slipping into the code when packages are installed in non-editable mode. Off the top of my head,
GeoipsListScripts,GeoipsTestScript, and the initial implementation ofGeoipsRunwill fail if installed in non-editable mode. This is because test scripts will not be available as they are in editable mode. We should search the codebase for any reference ofresources.files(<pkg_name>) / "../"and rethink whether or not this functionality is needed, as it won't work for a non-editable installed package.Code to demonstrate issue
GeoipsListUnitTestsGeoipsListScriptsGeoipsRun(this will not matter after #465 is merged)GeoipsTestScriptGeoipsTestLintingChecklist for Completion
resources.files(<pkg_name>) / "../"