Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix warnings when loading tilesfile #101

Merged
merged 1 commit into from
Feb 19, 2019
Merged

fix warnings when loading tilesfile #101

merged 1 commit into from
Feb 19, 2019

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Feb 14, 2019

This PR is a followup to PR #98 which tried to cleanup the case where desimodel.io.load_tiles() is given a tilesfile option that exists both locally and in $DESIMODEL/data/footprint/ ($DESIMODEL wins, albeit with a warning).

The implementation in PR #98 printed incorrect warnings about collisions even when tilesfile was given with a full path and didn't exist in $DESIMODEL, e.g.:

In [1]: !pwd
/global/cscratch1/sd/sjbailey/desi/fiberassign/refactor

In [2]: import desimodel.io

In [3]: tiles = desimodel.io.load_tiles(tilesfile='/global/cscratch1/sd/sjbailey/desi/f
   ...: iberassign/refactor/tiles-lite.fits')
/global/common/software/desi/cori/desiconda/20180709-1.2.6-spec/code/desimodel/master/py/desimodel/io.py:187: UserWarning: File "/global/cscratch1/sd/sjbailey/desi/fiberassign/refactor/tiles-lite.fits" in $DESIMODEL/data is shadowed by a local file.  Choosing $DESIMODEL file.
  warnings.warn(msg)

Despite the warning message saying that it was going to pick the $DESIMODEL file (which doesn't exist), it did correctly load the local file.

This PR updates the logic to avoid the incorrect warning. Summarizing:

  • if tilesfile isn't given, use $DESIMODEL/data/footprint/desi-tiles.fits
  • if tilesfile includes a path, use that without also checking $DESIMODEL since the full path is specifying a very specific file and we should just use that
  • if tilesfile doesn't include a path, it is potentially ambiguous about whether the user intended a local copy or a copy in $DESIMODEL, so check both. If there is a file by that name both locally and in $DESIMODEL/data/footprint/, print a warning and use the $DESIMODEL version (use "./desi-tiles.fits" if you want the local copy to win).

@tskisner
Copy link
Member

Looks good, thanks for fixing.

@sbailey sbailey merged commit 8b40935 into master Feb 19, 2019
@sbailey sbailey deleted the tilesfile branch February 19, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants