Skip to content

1089 full shore importer#1102

Draft
mew-nsc wants to merge 20 commits into
developfrom
1089_full_shore_importer
Draft

1089 full shore importer#1102
mew-nsc wants to merge 20 commits into
developfrom
1089_full_shore_importer

Conversation

@mew-nsc

@mew-nsc mew-nsc commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

🧰 Issue

Fixes #1089

🚀 Overview:

Created an importer for the CSV output of the full shore / DNA format

🤔 Reason:

To allow Full Shore / DNA data to be imported into Pepys

🔨Work carried out:

  • Collected unclassified sample data
  • Parse format 1
  • Parse format 2
  • Confirmed units
  • Clarified outstanding questions around the format
  • Tests pass

Confirmations

  • I have chosen reviewers for my PR.
  • I have chosen an appropriate label for the PR, adding interactive_review if reviewers will need to see UI
  • I have extended/updated the documentation in \docs folder
  • Any database content changes (Create, Edit, Delete) are recorded in the Log/Changes tables
  • Any database schema changes are implemented via alembic revision transitions
  • I have completed the mandatory sections of this document.
  • I have deleted any unused sections.

@mew-nsc mew-nsc self-assigned this Dec 7, 2021
@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 7, 2021 07:55 Inactive
@codecov

codecov Bot commented Dec 7, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1102 (928eb23) into develop (bb8d8b5) will increase coverage by 0.30%.
The diff coverage is 98.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1102      +/-   ##
===========================================
+ Coverage    80.03%   80.33%   +0.30%     
===========================================
  Files          110      111       +1     
  Lines        11889    12087     +198     
===========================================
+ Hits          9515     9710     +195     
- Misses        2374     2377       +3     
Impacted Files Coverage Δ
importers/full_shore_importer.py 98.71% <98.71%> (ø)
importers/wecdis_importer.py 97.68% <0.00%> (-0.94%) ⬇️
pepys_import/resolvers/command_line_resolver.py 94.60% <0.00%> (+0.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60ad4cd...928eb23. Read the comment docs.

Comment thread importers/full_shore_importer.py Outdated
state = datafile.create_state(data_store, self.platform, sensor, timestamp, self.short_name)

location = Location(errors=self.errors, error_type=self.error_type)
lat_degs = float(lat_token.text) * (180 / math.pi)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It may be neater to use Python's math.degrees() function (as we do in the eag_importer). This will make it easier to track instances where we're converting from radians to degrees.

@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 10, 2021 08:18 Inactive
@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 10, 2021 08:19 Inactive
@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 10, 2021 11:15 Inactive
@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 14, 2021 16:25 Inactive
@IanMayo IanMayo temporarily deployed to pepys-timeli-1089-full--chlfik December 16, 2021 12:47 Inactive
@mew-nsc

mew-nsc commented Dec 16, 2021

Copy link
Copy Markdown
Contributor Author

This PR is left open as a draft PR pending answers about the units that the format uses. The code as written assumes some units:

  • Heading/bearing units are unclear. Implementation assumes radians
  • Speed units are unclear. Implementation assumes knots
  • Elevation units are unclear. Implementation assumes metres
  • Latitude/Longitude units are unclear. Implementation assumes decimal radians

These will need confirming before this gets merged.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Importer for Full Shore format

2 participants