-
Notifications
You must be signed in to change notification settings - Fork 60
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
Configure warnings #319
Configure warnings #319
Conversation
@nden, despite the appearance of the diff, this is a pretty small change. I'd like to merge it if you have no objections. It should not cause any more warnings to be produced by the JWST pipeline. |
@drdavella This looks good to me. I have one suggestion. If so, the default of If you agree, just set the default of |
@nden I've been trying to clarify my thoughts on this issue. I think I need to add some documentation that describes these changes and the philosophy behind them. In the meantime, here are my current thoughts (which maybe can eventually be converted into documentation in some form): There are really two different stages at which warnings can occur:
Also, tag classes now have two different attributes related to versioning:
There are also now two levels of warning control:
The During schema validation, we detect mismatched tag versions. For example, if I read an ASDF file that references The During tag conversion, we determine whether a particular schema can be successfully converted to a custom Python data structure by a particular tag implementation. Continuing with the example above, if we encounter The TL;DR: It is useful and a good idea to explicitly define |
This is to make sure that warnings occur even when there are tag classes for a particular tag, as long as that tag does not represent the latest known version of that tag. The purpose of this commit is to prevent `supported_versions` from having an affect on YAML validation warnings. This also refactors the warning handling logic to consolidate and reuse code.
@nden reported that the use of |
OK, so really the version_mismatch warning is of no interest to end users. I think this can merged. I'll open a separate issue about |
I'm not sure it will never be of use to end users, but I think the warnings are of a sufficiently minor nature that they should be have to be explicitly enabled (maybe think of it like |
This PR does two things:
@nden, this seems like the right way to handle the large number of version mismatch warnings in jwst in the long term. However, I think that unrecognized tag warnings should be enabled by default, and there are a few other more serious warnings which are still not possible to silence at all.
Also, while this resolves #313 for the most part, it does not add a module-level configuration mechanism. There are too many questions about precedence that make it a little too complicated to implement without sufficient motivation. Please let me know if you disagree.