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 target label display #38

Merged
merged 3 commits into from
Apr 14, 2019
Merged

Conversation

ziyaointl
Copy link
Member

This is a simple fix to address issue #36

@ziyaointl
Copy link
Member Author

BTW I feel like this is a little hacky, since desinames list can have only one item, whereas bgsnames and mwsnames can have more than one. That being said, this fix does seem to work and is quite simple. Feel free to chime in if anyone has better ideas!

@dstndstn
Copy link
Member

Ahh, sorry for not explaining the problem better! Some targets can actually have, say, both QSO and LRG bits set, or QSO and ELG. They're not mutually exclusive, and targets that have more than one of those top-level bits are important to know about.

Maybe the simplest implementation would be a "veto" dictionary -- if you have bit ELG, remove the ELG_NORTH and ELG_SOUTH bits, and similarly for QSO, LRG, etc.

@ziyaointl
Copy link
Member Author

Hi Dustin,

Thanks for the input! I came up with two slightly different approaches. The first approach utilizes a veto dictionary. It handles more cases than the second approach: e.g. we can now filter out 'BGS_ANY' if any of the other BGS names exist.

# If any of the names in value exists, remove the key in bitnames
# Example: if 'BGS_FAINT' exists, remove 'BGS_ANY'
bitnames_veto = {
    'ELG_SOUTH': ['ELG'],
    'ELG_NORTH': ['ELG'],
    'QSO_SOUTH': ['QSO'],
    'QSO_NORTH': ['QSO'],
    'LRG_NORTH': ['LRG'],
    'LRG_SOUTH': ['LRG'],
    'BGS_FAINT_NORTH': ['BGS_FAINT'],
    'BGS_FAINT_SOUTH': ['BGS_FAINT'],
    'BGS_BRIGHT_NORTH': ['BGS_BRIGHT'],
    'BGS_BRIGHT_SOUTH': ['BGS_BRIGHT'],
    'BGS_ANY': ['BGS_FAINT', 'BGS_BRIGHT', 'BGS_FAINT_NORTH',
                'BGS_BRIGHT_NORTH', 'BGS_FAINT_SOUTH', 'BGS_BRIGHT_SOUTH',
                'BGS_KNOWN_ANY', 'BGS_KNOWN_COLLIDED', 'BGS_KNOWN_SDSS',
                'BGS_KNOWN_BOSS'],
    'MWS_ANY': ['MWS_MAIN', 'MWS_WD', 'MWS_NEARBY', 'MWS_MAIN_VERY_FAINT'],
}
for name in bitnames[:]:
    if any([better_name in bitnames for better_name in bitnames_veto.get(name, [])]):
        bitnames.remove(name)

The second approach is simpler. It loops through the names while removing other names that has it as their prefix. This approach can handling thing like ELGs, but it won't work for things like BGS_ANY.

for name in bitnames[:]:
    bitnames = filter(lambda x: x == name or not x.startswith(name), bitnames)
    bitnames = list(bitnames)

Please let me know which one you would prefer, and I will commit the corresponding version. Thanks!

@dstndstn
Copy link
Member

Hi Mark,
Awesome! I think the more general veto version will be handy for other features we might want to add later. Go with that one?

@ziyaointl
Copy link
Member Author

Sounds good! I just committed the changes

@dstndstn dstndstn merged commit 294c9c7 into legacysurvey:master Apr 14, 2019
@dstndstn
Copy link
Member

It's live!

@ziyaointl ziyaointl deleted the desi-elg-label branch April 15, 2019 20:15
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.

2 participants