Skip to content

Resolve warnings in SQLAlchemy field defs #1963

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

Merged
merged 1 commit into from
May 9, 2025
Merged

Conversation

shreve
Copy link
Contributor

@shreve shreve commented Apr 29, 2025

Most notably, Grade.cell_type was defined as an association_proxy in the class, but was later overwritten with a column_property. Because the mapper had already been made aware of the association_proxy, this raised a warning. Grade.max_score also had a similar problem, with a column_property overwriting a regular @​property.

This commit fixes the warnings by replacing the in-class field definitions with field = None.

Additionally, remove duplicate definitions for cell_type_gradecell and cell_type_taskcell.

Fixes #1946

Most notably, Grade.cell_type was defined as an association_proxy in the
class, but was later overwritten with a column_property. Because the
mapper had already been made aware of the association_proxy, this raised
a warning. Grade.max_score also had a similar problem, with a
column_property overwriting a regular @Property.

This commit fixes the warnings by replacing the in-class field
definitions with `field = None`.

Additionally, remove duplicate definitions for `cell_type_gradecell` and
`cell_type_taskcell`.

Fixes jupyter#1946
Copy link
Contributor

Binder 👈 Launch a Binder on branch shreve/nbgrader/issue-1946

@brichet
Copy link
Contributor

brichet commented May 7, 2025

Thanks @shreve for working on this.

As far as I understand:

  • cell_type and max_score attributes were always overwritten
  • cell_type_from_taskcell and cell_type_from_gradecell were similar to cell_type_taskcell and cell_type_gradecell, respectively, and used only to set cell_type

If so, your change looks good to me.

@shreve
Copy link
Contributor Author

shreve commented May 8, 2025

That is my understanding as well.

  • cell_type and max_score were both always overwriting the already-mapped methods/attrs in the classes, and this was detected by SQLAlchemy and firing off a soon-to-be-an-error warning.
  • cell_type_from_gradecell and cell_type_from_taskcell were exactly the same code as their non-from counterparts.

@shreve
Copy link
Contributor Author

shreve commented May 8, 2025

I guess this is more maintenance than bugfix because it's cleanup and removal of a deprecation warning.

Copy link
Contributor

@brichet brichet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @shreve

@brichet brichet changed the title fix: Resolve warnings in SQLAlchemy field defs Resolve warnings in SQLAlchemy field defs May 9, 2025
@brichet brichet merged commit ab7ef18 into jupyter:main May 9, 2025
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some code uses constructs that will fail under Python 3.12
2 participants