Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using DyadicInterval.to_dyadic_intervals() with RealInterval throws an error #95

Open
vchhabra-turing opened this issue Sep 5, 2024 · 2 comments
Assignees

Comments

@vchhabra-turing
Copy link

vchhabra-turing commented Sep 5, 2024

Describe the bug
Passing a RealInterval into DyadicInterval.to_dyadic_intervals() throws a TypeError.

To Reproduce

The code snippet below reproduces the error.

import roughpy as rp

interval = rp.RealInterval(0,1)
dyadic_intervals = rp.DyadicInterval.to_dyadic_intervals(interval)
print(dyadic_intervals)

Expected behavior
I expected a list of DyadicIntervals to be returned, instead I get:

TypeError: to_dyadic_intervals(): incompatible function arguments. The following argument types are supported:
    1. (interval: roughpy._roughpy.Interval, resolution: int, interval_type: roughpy._roughpy.IntervalType) -> List[roughpy._roughpy.DyadicInterval]
    2. (inf: float, sup: float, resolution: int, interval_type: roughpy._roughpy.IntervalType) -> List[roughpy._roughpy.DyadicInterval]

Invoked with: RealInterval(inf=0.000000, sup=1.000000, type=clopen)

Screenshots
Screenshot 2024-09-05 at 20 57 37

Desktop (please complete the following information):

  • OS: Linux (ubuntu 24.04)
  • Python 3.12.3 [GCC 13.2.0]
@vchhabra-turing
Copy link
Author

vchhabra-turing commented Sep 12, 2024

Update: I realised the issue is that 1) I wasn't passing in the resolution argument, and 2) the IntervalType needs to be passed in explicitly. For example, with interval = rp.RealInterval(0,100) both rp.DyadicInterval.to_dyadic_intervals(interval) and rp.DyadicInterval.to_dyadic_intervals(interval, 2) will return the above type error, however

rp.DyadicInterval.to_dyadic_intervals(interval, 2, rp.IntervalType.Clopen)

will run as intended. Also, the overload does also work similarly:

rp.DyadicInterval.to_dyadic_intervals(0,100,2, rp.IntervalType.Clopen)

The last two both (correctly) return the same result:
[Interval(inf=0.000000, sup=64.000000, type=clopen), Interval(inf=64.000000, sup=96.000000, type=clopen), Interval(inf=96.000000, sup=100.000000, type=clopen)]

This requirement for the interval type and resolution are not documented.

@inakleinbottle
Copy link
Contributor

OK so it seems the problem here is that the "optional" interval type argument does not have default argument (making it mandatory). The default value for the argument should be set to IntervalType::Clopen, which is the only interval type that is available right now.

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

When branches are created from issues, their pull requests are automatically linked.

3 participants