Skip to content

updating code examples for correct imports#5

Open
vgiralt wants to merge 6 commits into
caltechads:masterfrom
vgiralt:master
Open

updating code examples for correct imports#5
vgiralt wants to merge 6 commits into
caltechads:masterfrom
vgiralt:master

Conversation

@vgiralt
Copy link
Copy Markdown

@vgiralt vgiralt commented Mar 16, 2026

I found that most code usage examples did the wrong import omiting ".models" . This PR fixes them. Thanks for a very useful project.

@vgiralt
Copy link
Copy Markdown
Author

vgiralt commented Mar 16, 2026

The second commit includes all changes needed for full Django Admin funcionality. Admin did not even start with the code as installed with pip or downloaded from GitHub.

@cmalek
Copy link
Copy Markdown
Collaborator

cmalek commented Mar 17, 2026

Thanks for your PR! Your additions are very welcome!

I need a few things from you to be able to merge this:

  • Please remove all the "# vgiralt *" comments. You will get credit in the Changelog and the git commit history
  • Please add docstrings to all methods and attributes you added using Sphinx Napoleon format; you did do a good job on most, but you missed a few
  • Move all imports to the top of the file they are in
  • Please ensure there are no linter warnings/errors or mypy errors if you have not already done so
  • Update the demo application in sandbox to enable and demonstrate the admin functionality
  • Add tests to the test suite for the admin functionality, using pytest
  • Document the admin functionality in the project documentation, with examples, and any caveats

@vgiralt
Copy link
Copy Markdown
Author

vgiralt commented Mar 17, 2026

Thanks for your PR! Your additions are very welcome!

I'm happy to contribute to something I find quite useful!

I need a few things from you to be able to merge this:

I'll do my best to comply. Although I've been using Python since 1992 and Django since 2008 and I did my first contribution to an Open Source project back in 1993 to Wietse Venema's tcp_wrappers, I have to confess that I'm not so proficient in GitHub and this might be my first (maybe second) PR :-) I might need some hand holding :-)

* Please remove all the "# vgiralt *" comments.  You will get credit in the Changelog and the git commit history

No problem with that, it's not for fame, it's an ancient tradition I instituted thirty years ago when I started leading the university sysadmin team: "leave traces of what you touch so others know who to chase if something breaks" ;-)

* Please add docstrings to all methods and attributes you added using Sphinx Napoleon format; you did do a good job on most, but you missed a few

I'll check and correct (and learn Sphinx Napoleon format on the way)

* Move all imports to the top of the file they are in

Strange, that's what I usually do most of the time. I'll check.

* Please ensure there are no linter warnings/errors or mypy errors if you have not already done so

I think I've used blue (I've interacted with Barry Warsaw long ago) on the code, but I might have forgotten. Is blue ok?

* Update the demo application in sandbox to enable and demonstrate the admin functionality

Ok. Give me some time for that, as I will have to get acquainted with the demo application before.

* Add tests to the test suite for the admin functionality, using pytest

That's probably going the part I'll nedd some hand holding, pytest has been my "pending subject" for quite some time.

* Document the admin functionality in the project documentation, with examples, and any caveats

OK. Any preferred location for this?

@cmalek
Copy link
Copy Markdown
Collaborator

cmalek commented Mar 17, 2026

Welcome, then, brother! You and I have similar backgrounds. This is my 31st year at Caltech, and I also ran the central sysadmin team for Caltech for many years. For the last 20 years I've been running a DevOps/software engineering team and this is one of our open source packages.

  • Please do use ruff and mypy. That is our stack here, and I don't want to have to do more fixes after you are done. I've added them to the uv dev dependencies so if you do uv sync --dev you will get ruff and mypy installed
  • For the docs, add it as a new document doc/source/overview/admin.rst, and add that to the "Overview" table of contents in doc/source/index.rst

I want to warn you that the current pypi version has some bugs in it so we're not using it in production. I haven't had time to track them down because we're slammed here.

@vgiralt
Copy link
Copy Markdown
Author

vgiralt commented Mar 18, 2026

I hope I've done it well. I confess I've cheated because a friend of mine (Pythonist and sysadmin) had been insisting in showing off Claude for some time and we had to meet today, so, we have used his paid license for me to learn about Sphinx Napoleon and unit tests in one morning. I confess that I'm impressed. I now (more or less) understand the format thanks to Claude's comments and have a better understanding of tests creation for Django :-) Docs and demo will be hand crafted :-) ;-)

@vgiralt
Copy link
Copy Markdown
Author

vgiralt commented Mar 18, 2026

Claude's review of comments have detected errors in areas of the code I have not touched. I'm more than happy to apply them if you give me green light, meanwhile I'll refrain to touch anything outside my contributions. As mypy refuses to run in my system because it does not like types.py (error below) we also asked Claude to be so kind to run it for us and it gave us a report with needed changes. I'm attaching a PDF (in Spanish, as my friend chats with the thing in Spanish) with the reports, including the results of running the tests for Django Admin integration.

It's been a good for me to decide to contribute to django-ldaporm as I'm learning interesting things :-)

Error:

mypy: "types.py" shadows library module "types"
note: A user-defined top-level module with name "types" is not supported

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

Successfully merging this pull request may close these issues.

2 participants