Add support for astropy registry priorities #757
Conversation
|
|
||
| def decorator(func): | ||
| io_registry.register_reader(label, dtype, func) | ||
| if _astropy_has_priorities(): |
There was a problem hiding this comment.
Is this check really necessary? If a user-defined reader/writer has no priority, seems we can just default to the lowest priority instead of resorting to reflection?
There was a problem hiding this comment.
Older versions of io_registry.register_reader won't accept the additional argument, so if we don't do this check, we'll get exceptions when trying to add a priority. Once all supported versions of astropy accept priorities, we can drop the check.
|
@aragilar it'd be great to also have a simple loader priority test. |
aragilar
force-pushed
the
add_new_astropy_priority_support
branch
from
24308bc to
5885ed9
Compare
|
I've added a test for the priorities, which should would for both astropy versions with and without priority support. Not sure if you want to wait until 4.3 comes out before merging this or not. |
aragilar
force-pushed
the
add_new_astropy_priority_support
branch
from
186fa22 to
562aa4c
Compare
aragilar
force-pushed
the
add_new_astropy_priority_support
branch
from
562aa4c to
2ff82ff
Compare
|
I've rebased this on top of main so that the fixes in #833 are included (so everything passes). Now that 4.3 is out, did we want to have another look at this? |
2 participants
This moves from the old priority system, which does not work, to the proposed system in astropy/astropy#11214. This would close #171 and #584.
This lacks direct tests, as monkeypatching the registry will probably hide problems, but I'm open to other ideas for explicitly testing for this.