-
Notifications
You must be signed in to change notification settings - Fork 18
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
database in mapping report #289
Comments
Thanks @IanSudbery Should this be reported in the Best regards, |
Is this issue related with this one? If so, have you tried the proposed solution? |
CGATReport and CGATPipelines are best considered separate tools, where the former is used by the latter. report_sql_backend makes sense for CGATReport, as it describes better than "database" what is needed (i.e. not a NoSQL database). While in CGATPipelines we have made the decision to use SQL database, so "database" is sufficient. Renaming it to "report_sql_backend" would also not be quite right, as the database is the primary data repository for the database. It is not just used for report building for CGAT, but also, for example, in notebooks or within the Pipeline itself. |
I agree that I don’t think CGATReport should change.
However, having to add the path to the database in two different places in
production pipelines is tiresome, particularly when it isn’t a setting that
is present to be set by default in the pipeline.ini
In other pipelines, the base sub-class of TrackerSQL passes database_name
to the constructor. I think this is a good solution.
|
@IanSudbery should this be closed? |
Many pipelines are still not setting the SQL backend correctly. A brief check shows that at least mapping and readqc are not: Mapping uses the default CGATReport locations. |
Ok, im not really best to solve this as I always found CGAT report an enigma. Just so you are aware we are hoping to transition away from CGAT report and rely on multiqc for generic QC visualisation and bespoke reporting using Rmarkdown or Jupiter notebook in the future. |
The location of the database for pipelines is set by
database_name
in the ini. However, by defaultCGATReport
looks for the database name inreport_sql_backend
. Some pipelines bypass this by passingbackend=PARAMS["database_name"]
to theTrackerSQL
constructor in their pipeline specific subclassing ofTrackerSQL
.I don't know if CGATReport should look for
database_name
for report trackers should passdatabase_name
, but one of these should happen else the report can't find the database unless it is calledcsvdb
and is in the current directory.The text was updated successfully, but these errors were encountered: