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

Fix error when altering Part table that uses "master" keyword #991

Merged
merged 12 commits into from
Mar 2, 2023

Conversation

rly
Copy link
Contributor

@rly rly commented Feb 9, 2022

Fix #936.

The core of the issue appears to be that the "master" keyword in the Part table definition does not match the name of a table in context. So in this PR, I update alter only for Part tables so that "master" refers to the master table in context. There may be a more elegant way to fix this issue, but this seems to do the trick.

@rly
Copy link
Contributor Author

rly commented Feb 9, 2022

This solution does not handle the case of a part table using the "master" keyword in a more complex way, though.

@schema
class Probe(dj.Manual):
    definition = """
    id: int
    """

    class Shank(dj.Part):
        definition = """
        -> master
        """

    class Electrode(dj.Part):
        definition = """
        -> master.Shank
        """

datajoint/user_tables.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@guzman-raphael guzman-raphael left a comment

Choose a reason for hiding this comment

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

@rly Thank you for contributing to the fix! 💪

We appreciate your patience with our review but we've finally opened up some bandwidth to work through our PR queue.

Suggesting an alternative approach.

tests/test_alter.py Outdated Show resolved Hide resolved
datajoint/user_tables.py Outdated Show resolved Hide resolved
datajoint/user_tables.py Outdated Show resolved Hide resolved
docs-parts/intro/Releases_lang1.rst Outdated Show resolved Hide resolved
@rly
Copy link
Contributor Author

rly commented Feb 23, 2023

Thanks for taking a look at this PR. This issue comes up in the Frank Lab often.

Syntax, integration, and style checks pass. Thanks for the comprehensive developer documentation!

Copy link
Collaborator

@guzman-raphael guzman-raphael left a comment

Choose a reason for hiding this comment

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

@rly Np. 🍻

Most of all, thanks to contributors like you for your help! LGTM

@guzman-raphael guzman-raphael merged commit 3320e22 into datajoint:master Mar 2, 2023
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.

alter support for part tables
2 participants