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.

Sign up for GitHub

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

Jump to bottom

feat: import samples additional attributes - flash point and refractive index #2214

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

feat: import samples additional attributes - flash point and refractive index #2214

wants to merge 6 commits into from

Conversation

adambasha0
Copy link
Contributor

@adambasha0 adambasha0 commented Oct 15, 2024 •
edited
Loading

  • rather 1-story 1-commit than sub-atomic commits

  • commit title is meaningful => git history search

  • commit description is helpful => helps the reviewer to understand the changes

  • code is up-to-date with the latest developments of the target branch (rebased to it or whatever) => ⏩-merge for linear history is favoured

  • added code is linted

  • tests are passing (at least locally): we still have some random test failure on CI. thinking of asking spec/examples.txt to be commited

  • in case the changes are visible to the end-user,  video or screenshots should be added to the PR => helps with user testing

  • testing coverage improvement is improved.

  • CHANGELOG :  add a bullet point on top (optional: reference to github issue/PR )

  • parallele PR for documentation  on docusaurus  if the feature/fix is tagged for a release

@adambasha0 adambasha0 self-assigned this Oct 15, 2024
github-actions[bot]
github-actions bot reviewed Oct 15, 2024
View reviewed changes
app/api/helpers/report_helpers.rb
@@ -556,7 +556,10 @@ def build_sql_reaction_sample(columns, c_id, ids, checkedAll = false)
# deleted_at: ['wp.deleted_at', nil, 10],
molecule_name: ['mn."name"', '"molecule name"', 1],
molarity_value: ['s."molarity_value"', '"molarity_value"', 0],
molarity_unit: ['s."molarity_unit"', '"molarity_unit"', 0],
Copy link

Choose a reason for hiding this comment

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

Metrics/ModuleLength: Module has too many lines. [615/100]

lib/export/export_excel.rb
@@ -127,6 +127,12 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/AbcSize: Assignment Branch Condition size for filter_with_permission_and_detail_level is too high. [<13, 34, 28> 45.92/25]

lib/export/export_excel.rb
@@ -127,6 +127,12 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/CyclomaticComplexity: Cyclomatic complexity for filter_with_permission_and_detail_level is too high. [20/7]

lib/export/export_excel.rb
@@ -127,6 +127,12 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [32/30]

lib/export/export_excel.rb
@@ -127,6 +127,12 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/PerceivedComplexity: Perceived complexity for filter_with_permission_and_detail_level is too high. [22/8]

lib/import/import_sdf.rb
sample['dry_solvent'] = row['dry_solvent'] if row['dry_solvent'].present?
sample['purity'] = row['purity'] if row['purity'].present?
Copy link

Choose a reason for hiding this comment

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

Metrics/BlockLength: Block has too many lines. [89/25]

spec/lib/export/export_excel_spec.rb Outdated
sample_json['refractive_index'] = refractive_index
sample_json['molarity_value'] = '2.45'
sample_json['molarity_unit'] = 'M'
exporter.instance_eval('@headers =["melting pt", "flash point", "refractive index", "molarity"]', __FILE__, __LINE__)
Copy link

Choose a reason for hiding this comment

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

Layout/LineLength: Line is too long. [123/120]

spec/lib/import/import_sdf_spec.rb Outdated
rows: [{
'inchikey' => 'DTHMTBUWTGVEFG-DDWIOCJRSA-N',
'molfile' => Rails.root.join('spec/fixtures/mf_with_data_01.sdf').read,
'description' => "MOLECULE_NAME\n(R)-Methyl-2-amino-2-phenylacetate hydrochloride ?96%; (R)-(?)-2-Phenylglycine methyl ester hydrochloride\n\nSAFETY_R_S\nH: 319; P: 305+351+338\n\nSMILES_STEREO\n[Cl-].COC(=O)[C@H](N)c1ccccc1.[H+]\n",
Copy link

Choose a reason for hiding this comment

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

Layout/LineLength: Line is too long. [241/120]

spec/lib/import/import_sdf_spec.rb Outdated
{ inchikey: 'DTHMTBUWTGVEFG-DDWIOCJRSA-N',
is_partial: false,
molfile_version: 'V2000',
}
Copy link

Choose a reason for hiding this comment

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

Layout/MultilineHashBraceLayout: Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.

spec/lib/import/import_sdf_spec.rb Outdated
{ inchikey: 'DTHMTBUWTGVEFG-DDWIOCJRSA-N',
is_partial: false,
molfile_version: 'V2000',
}
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

@github-actions GitHub Actions
Copy link

LCOV of commit f97123b during Continuous Integration #4003

Summary coverage rate:
  lines......: 65.6% (14117 of 21517 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@adambasha0 adambasha0 linked an issue Oct 15, 2024 that may be closed by this pull request
Add purity to chemical import feature and XLSX template #1644
Open
@github-actions GitHub Actions
Copy link

LCOV of commit b1fe655 during Continuous Integration #4019

Summary coverage rate:
  lines......: 65.7% (14133 of 21517 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

github-actions[bot]
github-actions bot reviewed Oct 16, 2024
View reviewed changes
lib/import/import_chemicals.rb Outdated
key = to_snake_case(column_header)
format_value = value.strip
def self.build_chemical_data(map_column, chemical, key, formated_value)

Copy link

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundMethodBody: Extra empty line detected at method body beginning.

@github-actions GitHub Actions
Copy link

LCOV of commit 7734b77 during Continuous Integration #4028

Summary coverage rate:
  lines......: 65.6% (14128 of 21530 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem marked this pull request as draft October 29, 2024 08:27
@adambasha0 adambasha0 force-pushed the enhance-import-samples-feature branch from 7734b77 to f0f7636 Compare November 5, 2024 15:55
github-actions[bot]
github-actions bot reviewed Nov 5, 2024
View reviewed changes
lib/export/export_excel.rb
@@ -127,6 +127,14 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/AbcSize: Assignment Branch Condition size for filter_with_permission_and_detail_level is too high. [<13, 35, 30> 47.9/25]

lib/export/export_excel.rb
@@ -127,6 +127,14 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/CyclomaticComplexity: Cyclomatic complexity for filter_with_permission_and_detail_level is too high. [21/7]

lib/export/export_excel.rb
@@ -127,6 +127,14 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [34/30]

lib/export/export_excel.rb
@@ -127,6 +127,14 @@ def filter_with_permission_and_detail_level(sample)
string.split(',').join(' - ')
elsif column == 'solvent'
extract_label_from_solvent_column(sample[column]) || ''
elsif column == 'refractive index'
Copy link

Choose a reason for hiding this comment

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

Metrics/PerceivedComplexity: Perceived complexity for filter_with_permission_and_detail_level is too high. [23/8]

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2024

LCOV of commit f0f7636 during Continuous Integration #4108

Summary coverage rate:
  lines......: 65.6% (14120 of 21515 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@adambasha0 adambasha0 force-pushed the enhance-import-samples-feature branch from f0f7636 to 0c7a6b7 Compare November 5, 2024 16:46
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2024

LCOV of commit 0c7a6b7 during Continuous Integration #4109

Summary coverage rate:
  lines......: 65.6% (14117 of 21511 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@adambasha0
Copy link
Contributor Author

adambasha0 commented Nov 6, 2024 •
edited
Loading

enhancement to import fields like Location, Melting Point, Boiling Point, flash point, refractive index, density, and molarity and chemical volume with units, and fix existing bugs.

@adambasha0
Copy link
Contributor Author

Chemotion.dev.12.webm

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 6, 2024

LCOV of commit 485f974 during Continuous Integration #4115

Summary coverage rate:
  lines......: 65.6% (14117 of 21511 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem changed the title enhance import samples feature and spec tests and additional samples attributes (flash point and refractive index) feat: import samples additional attributes - flash point and refractive index Nov 7, 2024
@adambasha0 adambasha0 force-pushed the enhance-import-samples-feature branch from 485f974 to 17dda99 Compare November 7, 2024 14:57
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 7, 2024

LCOV of commit 17dda99 during Continuous Integration #4124

Summary coverage rate:
  lines......: 65.6% (14118 of 21510 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@TasnimMehzabin TasnimMehzabin TasnimMehzabin approved these changes

Assignees

@adambasha0 adambasha0

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add purity to chemical import feature and XLSX template
2 participants
@adambasha0 @TasnimMehzabin