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.

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

Fixed shared field names between measures and points #122

Merged
merged 6 commits into from
Aug 1, 2019

Conversation

jessemapel
Copy link
Contributor

Shared field names between measures and points are now called measure_ and point_

Until #121 is fixed, I have removed the changes to the to_isis method.

@jlaura jlaura requested a review from acpaquette June 19, 2019 17:53
# Some point and measure fields have the same name, so mangle them as point_ and measure_
point_cols = [self.point_field_map[attr] if attr in self.point_field_map else attr for attr in self.point_attrs]
measure_cols = [self.measure_field_map[attr] if attr in self.measure_field_map else attr for attr in self.measure_attrs]
cols = point_cols + measure_cols
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@jessemapel
Copy link
Contributor Author

This is going to have knock on effects in autocnet. So once this is merged, hold off on updating!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 44.825% when pulling 991ded1 on jessemapel:shared_fields into 99f0852 on USGS-Astrogeology:master.

@coveralls
Copy link

coveralls commented Jun 19, 2019

Coverage Status

Coverage decreased (-0.02%) to 45.267% when pulling b87c4ad on jessemapel:shared_fields into 99f0852 on USGS-Astrogeology:master.

@jessemapel
Copy link
Contributor Author

I'm not sure why coveralls is complaining about the coverage going down. I didn't modify the constructor property at all.

@jlaura
Copy link
Collaborator

jlaura commented Jun 26, 2019

@jessemapel Are we good to merge this? If so, anyone can do it. Reviewed and code looks good.

@jessemapel
Copy link
Contributor Author

@jlaura I want to wait until https://github.com/USGS-Astrogeology/autocnet/pull/349 is finished because this will break that.

@jlaura jlaura merged commit 6cbc2c1 into DOI-USGS:master Aug 1, 2019
Kelvinrr pushed a commit to Kelvinrr/plio that referenced this pull request May 26, 2022
* First pass at shared field names

* Removed write changes

* Removed print

* Added better testing. Fixed weird choosername problem
ladoramkershner pushed a commit to ladoramkershner/plio that referenced this pull request Jun 7, 2022
* First pass at shared field names

* Removed write changes

* Removed print

* Added better testing. Fixed weird choosername problem
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.

None yet

3 participants