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

Add support for "locked" and "pinned" header keywords #3837

Open
embray opened this issue Jun 10, 2015 · 7 comments
Open

Add support for "locked" and "pinned" header keywords #3837

embray opened this issue Jun 10, 2015 · 7 comments

Comments

@embray
Copy link
Member

embray commented Jun 10, 2015

Directly related to #3836, a feature I've been musing about for a while would be the ability to "lock" certain cards in a FITS header. A "locked" card is one whose value and comment cannot be changed, and which cannot be deleted unless it is explicitly unlocked. To support this, the Card class would gain new lock() and unlock() methods that lock and unlock the card, respectively.

In particular, some cards would be locked by default. These would include all "structural" keywords, as described in #3836. There are some corner cases where one might want to update these keywords manually (testing, fixing existing files, writing hand-crafted headers, etc.) so it would still be possible to manually unlock these cards. Also, for both convenience and to support existing code that needs it, a new protected=True keyword argument will be added to fits.open. The default value, True, locks structural keywords by default, while False leaves them unlocked (the current behavior). It would also be possible to change the default via a config option for those applications that need it.

Perhaps also to ease backwards compatibility the initial default would be to warn when changing a locked card, while in the future the default might be changed to raise an exception.

Similarly, a "pinned" card is a card whose position in the header cannot be changed. A "pinned" card also may not be deleted (but its value and/or comment may be changed). For example if a card is the first card in the header (i.e. SIMPLE) it may not be moved from that position, and no new cards may be inserted before it. If a card is pinned in place this restrictions any insertions above it (but not below it). This feature is less important than "locked", but still required for some cards, which would be pinned by default. A card may be both "locked" and "pinned" simultaneously, and pin() and unpin() methods will be included to change a card's pinned status. The protected argument to fits.open() affects both locked and pinned cards in the same way.

@embray
Copy link
Member Author

embray commented Jun 10, 2015

Related to the warning vs. exception issue, both lock() and pin() could support an exception=True/False argument that changes whether it should just result in a warning, or should raise an exception. That way this could be controlled on a per-card basis (certainly changing some is less severe than others, and this could also be useful for third-party applications that wish to lock certain cards).

As noted in the description, I would like to raise an exception whenever a structural keyword is changed. But for the sake of backward-compatibility maybe at least the first version to include this feature could default to warnings instead.

@embray
Copy link
Member Author

embray commented Jun 10, 2015

One other additional useful feature would be context managers called Card.unlocked() and Card.unpinned(), which would temporarily unlock/unpin cards, then re-lock/pin them. If the card is already unlocked/unpinned these context managers would still work and be no-ops. This would be useful, especially internally, where it might be needed to move protected cards around. An additional convenience would be a Header.unprotected() context manager that temporarily unlocks/unpins all cards.

@mdboom
Copy link
Contributor

mdboom commented Jun 10, 2015

Seems like a good solution to the problem of users trying to update structural keywords and expecting something magic to happen.

@embray
Copy link
Member Author

embray commented Jun 10, 2015

I feel like it could have other use cases as well, but that's obviously the primary motivation.

@mhvk
Copy link
Contributor

mhvk commented Jun 11, 2015

Sounds very good. I think I'd go for raising exceptions directly, but having the default for your new protected keyword argument be configurable. Hence, if people want backwards compatibility, they can get that with a single change.

@embray
Copy link
Member Author

embray commented Jun 11, 2015

I think for an initial version it's better to not have to make anyone manually enable backwards compatibility if at all possible. Later it can be made stricter.

@embray
Copy link
Member Author

embray commented Nov 2, 2017

This should have an io.fits label but it seems I can't add labels anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants