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 'tag' parsing #19

Merged
merged 4 commits into from
Apr 13, 2021
Merged

Conversation

CagtayFabry
Copy link
Contributor

After using tag in place of $ref more often (see asdf#814) I started to miss seeing the reference (+hyperlink) in the sphinx-asdf output that gets generated from $ref

Without going into details of the sphinx-asdf code I implemented a few snippets that (hopefully) parse the tag keywords similar to $ref. Some small examples I ran looked the way I expected.
Something like this:

definitions:
  root-gap:
    description: |
      Root gap
    tag: "tag:stsci.edu:asdf/unit/quantity-1.1.0"
    wx_unit: "mm"

Could this be added to sphinx-asdf?

@eslavich
Copy link
Contributor

eslavich commented Nov 2, 2020

Given that we're planning to allow wildcard patterns in the tag validator, is a link appropriate here? Or would we link only if there's no asterisk character in the tag value?

@CagtayFabry
Copy link
Contributor Author

Given that we're planning to allow wildcard patterns in the tag validator, is a link appropriate here? Or would we link only if there's no asterisk character in the tag value?

The glob asterisk sure breaks the link here so imo it seems reasonable to remove it in this case.
I did not want to fiddle with what _create_reference actually does, just get rid of the This node has no type definition (unrestricted) line when using the tag validator 😉

@eslavich eslavich self-requested a review November 3, 2020 15:48
@CagtayFabry
Copy link
Contributor Author

Given that we're planning to allow wildcard patterns in the tag validator, is a link appropriate here? Or would we link only if there's no asterisk character in the tag value?

I have removed the link in case tag is defined with any kind of asterisk usage @eslavich

@eslavich
Copy link
Contributor

eslavich commented Nov 5, 2020

Okay, thanks! Sorry for the delay, I need to either locate someone with the expertise to review this or learn something about the package myself. I'll bring it up at our morning meeting.

@CagtayFabry
Copy link
Contributor Author

Okay, thanks! Sorry for the delay, I need to either locate someone with the expertise to review this or learn something about the package myself. I'll bring it up at our morning meeting.

No hurry. I wasn't sure how much development is going on with sphinx-asdf
I am currently messing around to get to know (and adapt) the tool on my own fork for our project over at weldx but I thought some more generic asdf related things might be interesting for you as as well.

Copy link
Contributor

@eslavich eslavich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good. At some point we'll want to use the asdf library to resolve these but that can wait for now. I'll add an issue so we remember to do that at some point.

@eslavich eslavich merged commit 3eaae1e into asdf-format:master Apr 13, 2021
@CagtayFabry CagtayFabry deleted the add_tag_parsing branch April 13, 2021 18:22
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