search

m-lundberg / simple-pid

A simple and easy to use PID controller in Python
MIT License
792 stars 216 forks source link

Rename module from PID to pid #55

Closed rnestler closed 2 years ago

rnestler commented 2 years ago

This avoids shadowing the module with the re-export of the PID class. Fixes #53.

rnestler commented 2 years ago

I'm not sure if we should consider this a breaking change. While

from simple_pid import PID

will still work

from simple_pid.PID import PID

won't work anymore, but would need to be changed to

from simple_pid.pid import PID

So I guess if we interpret semver strictly this would mean we should do a 2.0.0 release.

m-lundberg commented 2 years ago

I actually played around with this a little bit the other day and found that

import simple_pid.PID as PID
print(PID)  # prints <class 'simple_pid.PID.PID'>

worked before the change but not after. I'm not exactly sure why though. But it seems like a very specific edge case, I doubt many people if any actually use the library like that.

I do think we should play it safe and make the next release 2.0.0 though, since this could in theory be breaking in some cases. I'll try to find time to look on some of the other PRs and issues soon and see if anything else should be included before I make a release.

rnestler commented 2 years ago

worked before the change but not after. I'm not exactly sure why though.

This seems to be something that shouldn't actually work, since you can't import classes directly (only with from .. import ...). It probably worked due to a weird side effect of PID being a module and being shadows by the PID class.