Remove `:type` lines now sphinx-autoapi supports typehints by ashb · Pull Request #20951 · apache/airflow

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb · 2022-01-19T13:37:24Z

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints and the :type lines -- which is good, as the ones in the doc strings are easy to get out of date!

The following settings have been set:

autodoc_typehints = 'description' -- show types in description (where previous :type used to show up)
autodoc_typehints_description_target = 'documented' -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure)
autodoc_typehints_format = 'short' -- Shorten type hints where possible, i.e. StringIO instead of io.StringIO

Before (https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/baseoperator/index.html#airflow.models.baseoperator.BaseOperator)

After

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

potiuk · 2022-01-19T13:46:48Z

HELL YEAH!

BTW. I guess the main reason you did it is to finally reach negative net number of lines changed in Airflow ?

ashb · 2022-01-19T13:47:40Z

I mean that was certainly part of the reason. A big part.

It just happens to help that it's also the right thing to do 😁

github-actions · 2022-01-19T13:48:28Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

ashb · 2022-01-20T13:04:53Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

potiuk · 2022-01-20T13:49:14Z

Looks like some spelling errors (aka ClassNames) coming in via the types now need to be added to dictionary

But images are ok, all tests pass and all looks good besides :)

ashb · 2022-01-20T16:27:27Z

Right, I think I've got all the type-induced spelling errors, unless one crept back in when rebasing 🤞🏻

potiuk · 2022-01-20T17:13:46Z

2 errors left @ashb !

potiuk · 2022-01-20T17:14:11Z

And conflicts with my cloud formation change :(

ashb · 2022-01-20T17:37:49Z

And conflicts with my cloud formation change :(

Fixed the errors.

Has your change merged? Ah, yes, conflicts already I see

Since we have no updated sphinx-autoapi to a more recent version it supports showing type hints in the documentation, so we don't need to have the type hints _and_ the `:type` lines -- which is good, as the ones in the doc strings are easy to get out of date! The following settings have been set: `autodoc_typehints = 'description'` -- show types in description (where previous `:type` used to show up) `autodoc_typehints_description_target = 'documented'` -- only link to types that are documented. (Without this we have some missing return types that aren't documented, and aren't linked to in our current python API docs, so this caused a build failure) `autodoc_typehints_format = 'short'` -- Shorten type hints where possible, i.e. `StringIO` instead of `io.StringIO`

Now that we are using the type hints in the docs, sphinxcontrib-spelling picks them up as words to be checked, so we have to ignore them. I've chosen to add the provider specific ones to local dictionary files rather than the global, as for example, `mgmt` is an error in most places, but not in some of the Azure provider.

ashb · 2022-01-20T21:07:32Z

@potiuk Any idea what failed with the 'Constraints" job https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true I don't actually see any error -- just some process exited with 1.

(I thought we only ran that on main, not on PRs?)

Ah, it's just the Push that only runs on main. That makes sense.

ashb · 2022-01-20T21:10:48Z

Oh there's the error much futher up https://github.com/apache/airflow/runs/4888325400?check_suite_focus=true#step:7:580

#29 8.356 The conflict is caused by:
#29 8.356 apache-airflow[devel-ci] 2.3.0.dev0 depends on sphinx<5.0.0 and >=4.4.0
#29 8.356 The user requested (constraint) sphinx==4.3.2

Ummmm, yeah, something is wrong about constraint generation now, as it's trying to generate new constraints while using the old file?!

potiuk · 2022-01-20T21:29:42Z

Rebase ?

ashb · 2022-01-20T21:30:53Z

Already up to date.

potiuk · 2022-01-20T21:34:39Z

Hmm. Will take a look in a bit

potiuk · 2022-01-20T22:52:15Z

We can merge the change now. The problem was because of one line missing in the latest refactor:
Fix is here: #21000

potiuk · 2022-01-20T22:52:57Z

BTW. This time the round number is mine ;)

ashb · 2022-01-21T09:24:33Z

We can merge the change now. The problem was because of one line missing in the latest refactor: Fix is here: #21000

Thanks, I had a feeling it would be something like this but didn't want to break main if it was a bigger problem.

ashb requested review from XD-DENG, bbovenzi, dstandish, ephraimbuddy, jedcunningham, jhtimmins, kaxil, mik-laj, potiuk, ryanahamilton and turbaszek as code owners January 19, 2022 13:37

boring-cyborgBot added area:API Airflow's REST/HTTP API area:core-operators provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:lineage area:Scheduler including HA (high availability) scheduler labels Jan 19, 2022

ashb mentioned this pull request Jan 19, 2022
Fix all Amazon Provider MyPy errors #20935
Merged

ashb removed request for bbovenzi and ryanahamilton January 19, 2022 13:39

ashb added kind:documentation and removed area:Scheduler including HA (high availability) scheduler area:API Airflow's REST/HTTP API provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:lineage area:core-operators labels Jan 19, 2022

potiuk approved these changes Jan 19, 2022
View reviewed changes

github-actionsBot added the full tests needed We need to run full set of tests for this PR to merge label Jan 19, 2022

potiuk approved these changes Jan 20, 2022
View reviewed changes

josh-fell mentioned this pull request Jan 20, 2022
Create CustomJob and Datasets operators for Vertex AI service #20077
Merged

ashb force-pushed the doc-types-from-hints branch from de76855 to 1149e63Compare January 20, 2022 16:26

ashb requested a review from vikramkoka as a code owner January 20, 2022 16:26

notatallshaw mentioned this pull request Jan 20, 2022
Present found conflicts when trying to resolve them pypa/pip#10258
Closed

ashb added 2 commits January 20, 2022 20:25

ashb force-pushed the doc-types-from-hints branch from 4685698 to b3aec74Compare January 20, 2022 20:25

potiuk merged commit 602abe8 into apache:mainJan 20, 2022

ashb deleted the doc-types-from-hints branch January 21, 2022 09:24

This was referenced Jan 24, 2022
[Part 2] Calendar to GCS Operator #20769
Merged
Remove :type directives from SqlToS3Operator#21079
Merged

josh-fell mentioned this pull request Feb 1, 2022
Add RedshiftDataHook #19137
Merged

potiuk mentioned this pull request Feb 5, 2022
Status of testing Providers that were prepared on February 05, 2022 #21348
Closed
54 tasks

josh-fell mentioned this pull request Mar 19, 2022
Kubernetes decorator #21077
Closed

ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Apr 11, 2022