Skip to content

Fix #511: prohibit direct inserts outside the make callback#514

Merged
eywalker merged 7 commits into
datajoint:masterfrom
dimitri-yatsenko:dev
Nov 13, 2018
Merged

Fix #511: prohibit direct inserts outside the make callback#514
eywalker merged 7 commits into
datajoint:masterfrom
dimitri-yatsenko:dev

Conversation

@dimitri-yatsenko

Copy link
Copy Markdown
Member

No description provided.

@coveralls

coveralls commented Oct 30, 2018

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.008%) to 88.574% when pulling e6295d0 on dimitri-yatsenko:dev into 0c9fac0 on datajoint:master.

@eywalker

Copy link
Copy Markdown
Contributor

I'm not entirely sure if I agree about adding this, at least without some discussions. I completely understand where this is coming from, but I want to make sure we flesh out all imaginable cases before we put in what would be a fairly strong restriction.

@dimitri-yatsenko

Copy link
Copy Markdown
Member Author

This feature is introduced due to a user issue when they did not understand how auto-populate worked. They first inserted into master and then called populate to have the parts populated. It took me a while to catch the bug because I did not imagine someone inserting into a computed table.

This feature is important for new users to understand how to properly use DataJoint.

If the user must insert directly, they can set allow_direct_insert=True to bypass the restriction.

@eywalker

Copy link
Copy Markdown
Contributor

Ok makes sense and ability to bypass was something I was going to request. Ok - I'd still like to think about this before I will fully accept. I also would prefer that you would create an issue describing this so that this PR can be considered as a fix for that issue.

@dimitri-yatsenko

Copy link
Copy Markdown
Member Author

It is a fix of issue #511

@eywalker

Copy link
Copy Markdown
Contributor

Ah I oversaw that.

Comment thread datajoint/table.py Outdated

@eywalker eywalker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs very good documentation but I agree will help promote more consistent use of computed tables.

@eywalker eywalker merged commit 9fdecfe into datajoint:master Nov 13, 2018
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.

3 participants