Skip to content

add two converter tutorials#11

Merged
perrygreenfield merged 4 commits intoasdf-format:masterfrom
perrygreenfield:converter_tutorials
Aug 28, 2021
Merged

add two converter tutorials#11
perrygreenfield merged 4 commits intoasdf-format:masterfrom
perrygreenfield:converter_tutorials

Conversation

@perrygreenfield
Copy link
Copy Markdown

As it says, two converter tutorials have been added. The first does not use schemas, the second does.

@perrygreenfield perrygreenfield requested review from eslavich and nden June 22, 2021 15:17
Comment thread Your_first_ASDF_converter.ipynb Outdated
},
{
"cell_type": "code",
"execution_count": null,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR is adding output to these old notebooks, is that intentional?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, I wonder how that happened.

"Python primative types (e.g., strings, numbers, booleans,\n",
"lists and dictionaries), as well as objects that have \n",
"serialization defined (e.g., astropy models and GWCS).\n",
"For example, if you define a class that has as one of\n",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's a subtlety to this, which is that the containing class's converter could be coded up to also handle the other class instance. Probably not worth getting into?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, I'd rather avoid exploding their heads this time around.

Comment thread Your_first_ASDF_converter.ipynb Outdated
Comment thread Your_first_ASDF_converter.ipynb Outdated
Comment thread Your_second_ASDF_converter.ipynb Outdated
Comment thread Your_second_ASDF_converter.ipynb Outdated
"metadata": {},
"outputs": [],
"source": [
"mkdir src/myschemas/resources"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is different from the structure created by the cookiecutter template, which puts the resources directory in the repo root.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hmmm, we seemed to have done it this way for rad. Should we have not done that?

Comment thread Your_second_ASDF_converter.ipynb
Comment thread Your_second_ASDF_converter.ipynb
@nden
Copy link
Copy Markdown

nden commented Jul 23, 2021

These look good. I only read them, haven't run the yet.
Perhaps remove tutorial from the two titles. It sounded to me as a tutorial on writing tutorials but that may be because it's Fri afternoon.

Also checkout the notebooks for the first tutorials to remove the changes that appear here.

@mcara
Copy link
Copy Markdown

mcara commented Aug 27, 2021

I think Your_*_ASDF_converter.ipynb should be renamed to something more meaningful.

@perrygreenfield perrygreenfield merged commit c067aa9 into asdf-format:master Aug 28, 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

Successfully merging this pull request may close these issues.

4 participants