Skip to content

FIREFLY-461: add VO resources to Firefly table.#946

Merged
loitly merged 3 commits intodevfrom
FIREFLY-461_table_resources
Mar 17, 2020
Merged

FIREFLY-461: add VO resources to Firefly table.#946
loitly merged 3 commits intodevfrom
FIREFLY-461_table_resources

Conversation

@loitly
Copy link
Copy Markdown
Contributor

@loitly loitly commented Mar 4, 2020

https://jira.ipac.caltech.edu/browse/FIREFLY-461

Add VO resources into Firefly table. Because we are flatten the VOTABLE hierarchy, this information will be lost when the table is exported or saved to disk.

Also:

  • allow resources and links to be injected via API

To test: https://irsawebdev9.ipac.caltech.edu/FIREFLY-461_table_resources/firefly/

- allow resources and links to be injected via API
@loitly loitly added Table Changes to table or table model API labels Mar 4, 2020
@loitly loitly requested a review from robyww March 4, 2020 22:46
@loitly loitly self-assigned this Mar 4, 2020
@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 4, 2020

Nice one!! I'm happy to see this coming soon.
One thing here just FYI- i've saved the table as VOTable (.vot) and validation failed. I don't how flexible we are with validated VOTables but interesting to see that units are not valid from that table. I thought unit was free form (???)

Validation report for http___web_ipac_caltech_edu_staff_roby_demo_WiseDemoTable_tbl.vot

28: W50: Invalid unit string 'DN'
<FIELD datatype="double" name="modeint" unit="DN"/>
^

37: W50: Invalid unit string 'e-/DEB ADU'
<FIELD datatype="double" name="debgain" unit="e-/DEB ADU"/>
^

38: W50: Invalid unit string 'e-/SUR ADU'
<FIELD datatype="double" name="febgain" unit="e-/SUR ADU"/>

@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 4, 2020

Also, i'm not sure if the table saved is ok in terms of link resources, the header looks like:

 xmlns="http://www.ivoa.net/xml/VOTable/v1.2">
<!--
 !  VOTable written by STIL version 3.2-debian (edu.caltech.ipac.table.io.VoTableWriter$VOTableWriterImpl)
 !  at 2020-03-04T22:58:26
 !-->
<RESOURCE>
<TABLE nrows="80">
<FIELD datatype="long" name="in_row_id">
<VALUES null='-9223372036854775808'/>
</FIELD>
<FIELD datatype="double" name="in_ra" unit="deg"/>
....
<FIELD datatype="int" name="qual_scan"/>
<LINK href="https://irsa.ipac.caltech.edu/?id="/>
<LINK value="ivoa-${band}" href="http://ivoa.net/?id=${band}"/>
<DATA>....
</DATA>

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 4, 2020

as VOTable (.vot) and validation failed

What did you use to validate the file?

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 4, 2020

i'm not sure if the table saved is ok in terms of link resources

This is existing code. I've only added the ability to inject links from the API. The saved links in the VOTABLE looks okay. Did validation failed? If so, which validator did you use.

@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 4, 2020

I used 'volint' tool from astropy-utils.

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 4, 2020

I used 'volint' tool from astropy-utils.

I am not familiar with this tool. I use an online XML validator and it validates okay. unit attribute of Field element is of type xs:token which should be able to take any string value.
We use starlink libraries with custom logic to write VO tables. If the resulting file is an issue, let's identify it and file a separate bug?

@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 4, 2020

ok.
Yes, probably 'volint' is using old version of the schema underneath so don't necessarily pay attention to it. Would be great to have an internal tool in Firefly to run validation but i can added later.

@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 4, 2020

For reference, STIL VOLINT doc is here: http://www.star.bris.ac.uk/~mbt/stilts/sun256/votlint.html

@gpdf
Copy link
Copy Markdown
Contributor

gpdf commented Mar 5, 2020

From VOTable 1.4: "Reference [3] contains the recommended rules for writing string representations of the unit strings, called VOUnits."

So this is SHOULD, not a SHALL, and anyway the votlint messages are W(arnings), not errors.

However, if this is just a demo table that we created internally, we might as well make sure that it's more conformant to the spirit of VOUnit.

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 5, 2020

The table in question is a sample WISE(?) IPAC table that @robyww used original in one of his demo code and is now used everywhere. @ejoliet if there's a request to clean up or modify the sample table, you should create a new ticket for it.

@ejoliet
Copy link
Copy Markdown
Contributor

ejoliet commented Mar 5, 2020

No, no cleanup expected. It's fine.

My point was that we could argue that Firefly 'should' export validated VOTable, within what validated table means. If warning are accepted, then fine.

If we are strict, we would have to look into how to validate exported data.
But then what would the tool do if we can't export data into 'valid' VOTable?
That's a bigger story to take into account right now, and definitively out of scope of this ticket but also out of scope of the near term priorities.

My. question is: do we need to make sure that the injected link from API gets added to the VOTable header accordingly?
If it's part of the PR, then we will need to get a source sample table that can be tested and validated without having to put more effort on the overall validation during export step.
What do you think?

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 5, 2020

My. question is: do we need to make sure that the injected link from API gets added to the VOTable header accordingly?

I answered it earlier. The links look good when saved as votable.

Copy link
Copy Markdown
Contributor

@robyww robyww left a comment

Choose a reason for hiding this comment

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

Code looks good. I think I found an issue parsing out the complete resource. I will attach the xml file that I am referencing to the ticket.

If you look at the resource snippet below, it is resouce -> groups -> param[3], it turns into

filterable: false
UCD: "obs.field"
name: "CIRCLE"
arraySize: "3"
units: "deg"
sortable: false
type: "double"

I don't see an xtype but I am not sure that matters. The real issue is I don't see a values object.
I would think it would look something like.

   .... edited out stuff ....
type: "double"
values: {
  MAX: [10.685001466285922, 41.25363121467171, 0.7002317445980466]  
}
 <RESOURCE type="meta" ID="soda-c3e4baa9-9b7b-491a-8129-c54e7e82cf56" utype="adhoc:service">
    <PARAM name="resourceIdentifier" datatype="char" arraysize="26" value="ivo://cadc.nrc.ca/caom2ops" />
    <PARAM name="standardID" datatype="char" arraysize="32" value="ivo://ivoa.net/std/SODA#sync-1.0" />
    <PARAM name="accessURL" datatype="char" arraysize="58" value="https://www.cadc-ccda.hia-iha.nrc-cnrc.gc.ca/caom2ops/sync" />
    <GROUP name="inputParams">
      <PARAM name="ID" datatype="char" arraysize="24" ucd="meta.id;meta.dataset" value="ad:CFHT/1250312o.fits.fz" />
      <PARAM name="RUNID" datatype="char" arraysize="16" ucd="" value="i9it86ymxfdlhqd0" />
      <PARAM name="POS" datatype="char" arraysize="*" ucd="obs.field" value="" />
      <PARAM name="CIRCLE" datatype="double" arraysize="3" ucd="obs.field" unit="deg" xtype="circle" value="">
        <VALUES>
          <MAX value="10.685001466285922 41.25363121467171 0.7002317445980466" />
        </VALUES>
      </PARAM>
      <PARAM name="POLYGON" datatype="double" arraysize="*" ucd="obs.field" unit="deg" xtype="polygon" value="">
        <VALUES>
          <MAX value="11.335862813772264 40.75492965204083 10.038755258824418 40.75155222459407 10.024294607387546 41.7494483393487 11.33998146387876 41.752588171618264" />
        </VALUES>
      </PARAM>
    </GROUP>
  </RESOURCE>

@loitly
Copy link
Copy Markdown
Contributor Author

loitly commented Mar 17, 2020

@robyww VALUES are added back into the table model. We're also displaying resources in File Upload and Table Options as well. Test link updated with the changes.

@loitly loitly merged commit c7292ee into dev Mar 17, 2020
@loitly loitly deleted the FIREFLY-461_table_resources branch March 17, 2020 19:55
@robyww robyww added this to the 2020.1 milestone Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Table Changes to table or table model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants