-
Notifications
You must be signed in to change notification settings - Fork 1
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
v0.5.0 #90
v0.5.0 #90
Conversation
# Conflicts: # pepdbagent/const.py # pepdbagent/db_utils.py
I notice there's not |
usually there's a newline between description and params in docstrings, but I see several docstrings that are missing that now. can you update your docstrings? eg: pepdbagent/pepdbagent/modules/annotation.py Lines 238 to 239 in f9c94a0
|
is this docstring correct? pepdbagent/pepdbagent/modules/project.py Line 27 in f9c94a0
If so I recommend this be named |
except KeyError: | ||
return "" | ||
|
||
|
||
class Projects(Base): | ||
__tablename__ = "projects" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add docstrings to all these classes? I cannot infer what they are used for.
|
||
class Samples(Base): | ||
__tablename__ = "samples" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing docstring
if found_prj.samples_mapping: | ||
for sample in found_prj.samples_mapping: | ||
_LOGGER.debug(f"deleting samples: {str(sample)}") | ||
session.delete(sample) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to delete more than 1 at time instead of looping like this?
eg, DELETE WHERE ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the easiest way how to do it using sqlalchemy that I found. Not sure if it's possible using declarative object
if found_prj.subsamples_mapping: | ||
for subsample in found_prj.subsamples_mapping: | ||
_LOGGER.debug(f"deleting subsamples: {str(subsample)}") | ||
session.delete(subsample) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here; this seems like an inefficient way to do this.
I think I've addressed all comments |
Retructured all db! Now we have 3 tables: projects, samples, and subsamples
One step closer to: