Skip to content

CADC-14486 RAFTS frontend#21

Open
szautkin wants to merge 5 commits intoopencadc:mainfrom
szautkin:CADC-14486-front-only
Open

CADC-14486 RAFTS frontend#21
szautkin wants to merge 5 commits intoopencadc:mainfrom
szautkin:CADC-14486-front-only

Conversation

@szautkin
Copy link

No description provided.

import os
from app.config import logger
from app.utils.validation import validate_ades_xml, extract_xml_info
from app.utils.conversion import convert_psv_to_xml
Copy link
Member

Choose a reason for hiding this comment

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

The iau-ades library already comes with a psvtoxml (https://github.com/IAU-ADES/ADES-Master/blob/master/Python/ades/psvtoxml.py#L521). We may want to just make use of that unless there is a good reason not to.

Copy link
Author

Choose a reason for hiding this comment

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

@at88mph we have a function here - > rafts/api/validator/app/utils/conversion.py
this >
async def convert_psv_to_xml(psv_file_path, xml_output_path):
"""
Convert a PSV file to XML format using the ADES converter.

Args:
    psv_file_path: Path to the PSV file
    xml_output_path: Path to save the resulting XML file

Returns:
    Tuple of (success, message)
"""
import ades.psvtoxml

return await _run_converter(
    ades.psvtoxml.__file__, psv_file_path, xml_output_path, "PSV to XML"
)

do you think we need to change here anything?

import os
from app.config import logger
from app.utils.validation import validate_ades_xml, extract_xml_info
from app.utils.conversion import convert_mpc_to_xml
Copy link
Member

Choose a reason for hiding this comment

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

The iau-ades library already comes with an mpc80coltoxml (https://github.com/IAU-ADES/ADES-Master/blob/master/Python/ades/mpc80coltoxml.py). We may want to just make use of that unless there is a good reason not to.

I could find no reference in the documentation for MPC files either. Do you know much about them or who will use them?

Copy link
Author

Choose a reason for hiding this comment

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

@at88mph we use it here -> rafts/api/validator/app/utils/conversion.py
this one -> async def convert_mpc_to_xml(mpc_file_path, xml_output_path):
"""
Convert an MPC 80-column format file to XML using the ADES converter.

Args:
    mpc_file_path: Path to the MPC 80-column file
    xml_output_path: Path to save the resulting XML file

Returns:
    Tuple of (success, message)
"""
import ades.mpc80coltoxml

return await _run_converter(
    ades.mpc80coltoxml.__file__, mpc_file_path, xml_output_path, "MPC 80-col to XML"
)

do you think we need to write it differently?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, the import statements are down below and I missed that. Thanks.

XML_PARSER = etree.XMLParser(resolve_entities=False)


async def validate_ades_xml(xml_file_path, validation_type):
Copy link
Member

Choose a reason for hiding this comment

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

The XML Validation is done in the library as well:
https://github.com/IAU-ADES/ADES-Master/blob/master/Python/ades/validate.py#L39

Can that be reused to minimize what we're supporting?

Copy link
Author

Choose a reason for hiding this comment

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

@at88mph we cannot use that script as is on the web server but our solution is based on its idea.

Copy link
Member

@at88mph at88mph left a comment

Choose a reason for hiding this comment

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

See inline comments.

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.

3 participants