Skip to content

Conversation

anishdixit-uw
Copy link
Collaborator

  1. Added 2 new files: dev_setup.sh which contains python commands to install required packages and develop ooipy and verify_setup.py which confirms OOIPY installation and returns path
  2. Added instructions on the step by step process to run these files and setup the environment in docs/source/install_instructions.rst

@John-Ragland John-Ragland self-requested a review July 7, 2023 17:40
@John-Ragland
Copy link
Member

Alright, when following the instructions, when running dev_setup.sh I get this error:

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
Retrieving notices: ...working... done
added /Users/jhrag/Code/ooipy_development/ooipy
completed operation for: /Users/jhrag/Code/ooipy_development/ooipy
usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

error: no commands supplied
Traceback (most recent call last):
  File "/Users/jhrag/Code/ooipy_development/ooipy/verify_setup.py", line 1, in <module>
    import ooipy
  File "/Users/jhrag/Code/ooipy_development/ooipy/ooipy/__init__.py", line 1, in <module>
    from _ooipy_version import version as __version__  # noqa
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named '_ooipy_version'

Also, another small thing. This installs python 3.11, which we don't officially support with unit tests. There probably isn't any error, but it might be nice to add the 3.11 unit tests and the support for python 3.11 I guess

@anishdixit-uw
Copy link
Collaborator Author

@John-Ragland

that seems strange because in the .sh file, I have put in a command to run the setup.py file.

I think the error is occuring because I am first running conda develop ooipy and then running setup.py.

Let me change the order in which those commands are being run and then we can check if it works

@John-Ragland
Copy link
Member

@John-Ragland

that seems strange because in the .sh file, I have put in a command to run the setup.py file.

I think the error is occuring because I am first running conda develop ooipy and then running setup.py.

Let me change the order in which those commands are being run and then we can check if it works

Alright yeah, changing the order fixed it in my environment

@anishdixit-uw
Copy link
Collaborator Author

Okay, I have committed the change with the swapped order here too

Copy link
Member

@John-Ragland John-Ragland left a comment

Choose a reason for hiding this comment

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

This looks great, I think we're ready to merge.

Copy link
Member

Choose a reason for hiding this comment

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

We could probably run verify_setup.py within the bash script.

And it might even be possible to check if the python path is the local directory within the python script, and then just print something like
'Development environment installation successful'

Copy link
Member

Choose a reason for hiding this comment

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

@anishdixit-uw, would you be able to add these changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@John-Ragland verify_setup.py already runs from within the bash script.

And it does print out the path of OOIPY installation, so is that what you are saying here or something different?

Copy link
Member

Choose a reason for hiding this comment

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

Also, could we change name lines for creating the environment to have the environment name be ooipy_dev?

conda create --name ooipy_dev

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@John-Ragland for now I assumed that the environment is already created, then the user clones the repo and then runs the bash script.

If we want to give in environment creation instructions ourselves, the documentation will have to be changed, so the new order would be:

  1. Clone repo
  2. Run bash script (env gets automatically created)

If this logic makes sense, I can make the changes, should be easily doable

@John-Ragland
Copy link
Member

Tagging the relevant issue #136

@John-Ragland John-Ragland merged commit f7581f6 into Ocean-Data-Lab:116_prerelease Jul 7, 2023
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.

2 participants