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

Categorising UCD status #4

Closed
mbtaylor opened this issue Jul 8, 2021 · 8 comments
Closed

Categorising UCD status #4

mbtaylor opened this issue Jul 8, 2021 · 8 comments
Assignees
Milestone

Comments

@mbtaylor
Copy link

mbtaylor commented Jul 8, 2021

@gmantele, I'm trying to write a function that reports a single category for a presented UCD. My current attempt looks like this:

        UCD pucd = UCDParser.parseUCD( ucd );
        if ( ! pucd.isAllValid() ) {
            return "BAD_SYNTAX";
        }
        else if ( ! pucd.isAllRecognised() ) {
            return "UNKNOWN_ATOM";
        }
        else if ( ! pucd.isFullyValid() ) {
            return "BAD_SEQUENCE";
        }
        else if ( ! pucd.isAllRecommended() ) {
            return "DEPRECATED";
        }
        else {
            return "OK";
        }

However, it's not doing what I want. In particular it never seems to report DEPRECATED, because deprecated words are flagged as not recognised as well as not recommended, which is not what I expected from the UCDWord javadocs:

jshell> UCDParser.parseUCD("time.expo").isAllRecognised();
$27 ==> false

jshell> UCDParser.parseUCD("time.expo").getErrors().next();
$28 ==> "1 DEPRECATED UCD word: \"time.expo (-> time.duration;obs.exposure)\"!"

Can you advise? Thanks.

@gmantele gmantele added this to the Ucidy-1.2 milestone Jul 15, 2021
@gmantele gmantele self-assigned this Jul 15, 2021
@gmantele
Copy link
Owner

I think, the confusion here comes from whether the present or past tense is used. Personally I use the past tense, hence: a deprecedated word is no more valid because of its deprecated status. Though, it was a recommended word, in the last supported version of UCDList-x.y.

I may be wrong. It is true that a deprecated word is an old IVOA recommended world and is grammatically valid and recognised (so I should probably fix this last point). But I am not sure it is still a recommended word....it is actually recommended to not use it anymore.

What makes more sense according to you?

Anyway, I am confused by your interpretation of ! pucd.isAllRecommended(). If you define and use your own UCD words (so, not IVOA official words), they are flagged as not recommended, but that does not mean they are deprecated.

@mbtaylor
Copy link
Author

You are right that my pucd.isAllRecommended() is wrong, I was trying to come up with something that would report deprecated words, but doing it badly.

I agree that deprecated words should not be recommended.

I was mostly confused by deprecated words not being recognised. If that gets resolved, then I can identify deprecated as (recognised && !recommended) - do you agree? - and do everything that I want. (In the mean time I'm checking for the string "DEPRECATED" in the error message, which seems to do the trick, so this fix is not urgent for me).

@gmantele
Copy link
Owner

I can identify deprecated as (recognised && !recommended) - do you agree?

No, sorry :-( For a UCD word officially defined by the IVOA (UCDList-x.y), this condition is enough. However, it is possible to define its own custom UCD words. So, a custom UCD word is recognised but not recommended (by the IVOA). It does not make it necessarily a deprecated word.

Actually, for what you want to achieve, I think you need a new function: something like UCD.containsDeprecated() or UCD.listDeprecated(). Any of these can anyway easily be reproduced as you have the list of UCDWords (cf UCD.iterator()). From this, you can inspect each UCDWord and know if one is deprecated or not (UCDWord.isDeprecated()). If you want, I can add either containsDeprecated() or listDeprecated() (or both).

@gmantele
Copy link
Owner

In the mean time I'm checking for the string "DEPRECATED" in the error message, which seems to do the trick, so this fix is not urgent for me

This DEPRECATED message comes actually from the detection of a UCDWord.isDeprecated() item in a UCD instance.

@mbtaylor
Copy link
Author

Got it. Using UCDWord.isDeprecated() is fine for me, I was looking in the wrong place for this functionality (the UCD instead of the UCDWord). Thank you!

@gmantele
Copy link
Owner

Ok :)

Having a containsDeprecated() or listDeprecated() in UCD, would it be also interesting?

@mbtaylor
Copy link
Author

It would fit in with the other UCD-level methods isAllRecommended, isAllValid etc; but I've written the code to work with the UCDWord-level method now, so I probably won't use them.

@gmantele
Copy link
Owner

FYI, I just added the followings functions in UCD:

  • containsDeprecated()
  • countDeprecated()
  • getDeprecated()

@mbtaylor mbtaylor closed this as completed Nov 3, 2021
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

No branches or pull requests

2 participants