Add requirements #54
Add requirements #54
Conversation
|
Is there another way to have discovered I needed to use How does discord.py use PyNaCl without it being imported? Does the library just access it directly? Is it alright to add it to the imports (with a note) to make |
| from discord.ext import commands | ||
| from discord.ext import tasks | ||
| from datetime import datetime, timedelta, timezone | ||
| import nacl #This import is not required, but provides `requirements.txt` clarity, and use of `pipreqs`. |
There was a problem hiding this comment.
Ah so this is how you ensure pynancl gets include in pipreqs? Neat
There was a problem hiding this comment.
It's a good way to do it, right? I didn't see documentation, just noticed how pipreqs worked, and went ahead with that.
Is there a better way to handle it?
There was a problem hiding this comment.
No I think this is a good way to do it. You could have just manually added it to the file but this will allow you to regenerate consistently for future changes.
| from discord.ext import commands | ||
| from discord.ext import tasks | ||
| from datetime import datetime, timedelta, timezone | ||
| import nacl #This import is not required, but provides `requirements.txt` clarity, and use of `pipreqs`. | ||
| from rainwaveclient import RainwaveClient #Command to upgrade the rainwaveclient api: pip install -U python-rainwave-client |
There was a problem hiding this comment.
This might be a good example of when to use TODO in your comments. This would be a great thing for your README
There was a problem hiding this comment.
Weird, on the PR home page its showing I selected lines 15/16 but on the files view I had just selected line 16. The comment on line 16 about how to upgrade the client specifically
There was a problem hiding this comment.
Why TODO instead of NOTE?
There was a problem hiding this comment.
That works great too! Just something to help find these in the future for those that don't have issues created yet
| aiocron==1.8 | ||
| discord.py==2.4.0 | ||
| python_rainwave_client==2024.2 | ||
| PyNaCl==1.5.0 #I got this by using `pip show pynacl`. If I hadn't known what would I do? |
There was a problem hiding this comment.
Is your comment asking how to get the version?
There was a problem hiding this comment.
It's a "hidden" requirement, it's not something that gets imported so pipreqs doesn't see it.
So let's pretend I'm working on someone elses program and trying to containerize it, but I don't see a hidden requirement like this; is there a good way to detect requirements?
There was a problem hiding this comment.
Well in reality we detect this one by simply trying to run it. The program throws the exception which is how you discovered it in the first place. Its up to the maintainer to ensure they have things like this in requirements in general I would say. For me this one is no different than ffmpeg or opus. This just happens to be a python import we need. Was weird pipreqs didn't get this by default though
There was a problem hiding this comment.
It's not required unless you are using voice with the bot, which is why it's not a flatout requirement.
Okay, so there's not a good solution in this instance, we just hope folks upstream keep things in order for us.
There was a problem hiding this comment.
Gotcha, so yeah we are at the mercy of the upstream maintainer for discord and their documentation to figure that out.
This PR adds a requirements.txt file, and reorganizes the imports section to make more sense.
Should close #26 #29