uranusjr commented on PR #29721:
URL: https://github.com/apache/airflow/pull/29721#issuecomment-1469628487

   The “missing” numpy registrations are intentional; those are aliases and can 
never be identified by the `isinstance` logic.
   
   ```pycon
   >>> import numpy
   >>> numpy.int_
   <class 'numpy.int64'>
   ```
   
   This is on a 64-bit machine; on 32-bit it’s aliased to `int32`. But the 
point is that no object would ever report itself as an instance of 
`numpy.int_`, only either one of the actual int classes.
   
   But IMO that’s beside the point either way. Missing entries is a problem no 
matter whether you register the types, you can equally miss adding a type to 
the per-module registry to adding one to the global registry. The only downside 
of putting all the registration in one module is when you miss registering _an 
entire submodule in `airflow.serialization.serializers`_, which IMO is very 
easy to catch (and can be automated with pre-commit if needed), and worthwhile 
considering how it simplifies implementing individual serializer modules in 
general (no need to consider import performance for each module since it’s 
handled by the global registry).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to