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

Python 3 for review #59

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Conversation

ChristinaB
Copy link

@ChristinaB ChristinaB commented May 21, 2019

Daniella - It looks like you have made a few updates since I forked. I can make those changes manually unless you suggest other steps for pull request. Do you want a different branch?


This change is Reviewable

@ChristinaB ChristinaB mentioned this pull request Nov 10, 2019
@aaarendt
Copy link

Just a note that I've successfully tested this for ICESat-2 2020 and it's been working great in Python 3. Thanks @deppen8 and @ChristinaB for your work on this. Let me know if I can offer help so this can get merged.

@emiliom
Copy link

emiliom commented Jul 15, 2020

Any chance this PR could be merged soon? Can I help in some way?

We are getting set to review applications for OceanHackWeek 2020. Applications closed today, and we're on an accelerated track. We haven't made a definitive decision to use Entrofy, but we'd like to. For various reasons (none good), we haven't used it in previous years.
Thanks!

@dhuppenkothen
Copy link
Owner

dhuppenkothen commented Jul 20, 2020

I'm really sorry I didn't get to this earlier. Thank you for doing the Python3 conversion. I have two comments:

  • Could you perhaps include a summary of the changes somewhere? There are a lot of changes that are formatting only, and it's really hard to sort through what changes beyond formatting were included.
  • I think the PR requires a little bit of cleaning up. It looks like perhaps some files were committed accidentally? There are several data files and notebooks for water hack week and geo hack week that were committed as part of the PR that I'm pretty sure shouldn't be there, as well as housekeeping files like ipython notebook checkpoints.

I think the presence of the latter files might raise some privacy concerns? I think I'd recommend updating your fork with the current master of entrofy, cloning a clean copy, make a branch, then copy the files with the relevant changes (e.g. entrofy/core.py), committing those changes, and making a new pull request? Happy to help with that if it turns out to be complicated!

@deppen8
Copy link
Contributor

deppen8 commented Jul 21, 2020

I can take a run at this tonight since it was my original PR to Christina's fork that kicked things off. I'll make a commit directly to entropy without the Black formatting fun :D

@emiliom
Copy link

emiliom commented Jul 21, 2020

Thank you @dhuppenkothen! And @deppen8 thanks so much for offering to clean up the original PR! Great to hear from you.

A "direct commit directly to entropy" sounds like a very interesting proposition 😉; maybe more interesting than a commit to entrofy.

@deppen8
Copy link
Contributor

deppen8 commented Jul 21, 2020

Autocomplete!!! :shakes fist:

To be fair though, most of my commits do increase entropy 😁

@deppen8
Copy link
Contributor

deppen8 commented Jul 21, 2020

See #63

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.

5 participants