TimeDelta includes some methods that don't apply #6828

Activity

1. astrofrog

   added

   time

   on Nov 10, 2017
2. bsipocz

   added

   Bug

   on Nov 10, 2017
3. mhvk commented on Nov 10, 2017

   

   mhvk

   on Nov 10, 2017

   Contributor

   More actions

   @taldcroft - these methods are inherited directly from Time. To avoid this, should we have a TimeBase from which both Time and TimeDelta inherit? It is rather painful in that then isinstance(..., Time) will no longer catch a TimeDelta. But I'm not sure how else to solve this.
4. Cadair commented on Nov 11, 2017

   

   Cadair

   on Nov 11, 2017

   Member

   More actions

   Could some combo of ABCs and subclass hooks fix the isinstance(..., TIme) work?
5. taldcroft commented on Nov 12, 2017

   

   taldcroft

   on Nov 12, 2017

   Member

   More actions

   Good point. The only question is if we are getting into "perfect is the enemy of good" territory (unless @Cadair can produce the appropriate magic).
6. astrofrog commented on Nov 12, 2017

   

   astrofrog

   on Nov 12, 2017

   Last edited by astrofrog

   MemberAuthor

   More actions

   @mhvk - just to check, why should isinstance(..., Time) catch TimeDelta? Those are two quite different things, so in fact isinstance(..., Time) returning True for TimeDelta seems like a bug to me :)
7. astrofrog commented on Nov 12, 2017

   

   astrofrog

   on Nov 12, 2017

   Last edited by astrofrog

   MemberAuthor

   More actions

   Just to clarify my comment, here's what I get with the datetime module:

   In [3]: from datetime import datetime
   
   In [5]: dt1 = datetime.now()
   
   In [6]: dt2 = datetime.now()
   
   In [7]: dt2 - dt1
   Out[7]: datetime.timedelta(0, 4, 731509)
   
   In [8]: isinstance(dt2 - dt1, datetime)
   Out[8]: False
   
   In [9]: isinstance(dt1, datetime)
   Out[9]: True

   There's really no reason TimeDelta should be as a subclass of Time (conceptually) so I think @mhvk's suggestion of a TimeBase makes sense.
8. mhvk commented on Nov 12, 2017

   

   mhvk

   on Nov 12, 2017

   Contributor

   More actions

   @astrofrog - I agree with you that conceptually they're different and it makes sense to have a TimeBase. It is just that this may break code, as it is not how it is implemented...
9. mhvk

   closed this as completedin #10656on Oct 3, 2020