search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
716 stars 38 forks source link

[REVIEW]: CleanX: A Python library for data cleaning of large sets of radiology images #3632

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@drcandacemakedamoore<!--end-author-handle-- (Candace Makeda Moore) Repository: https://github.com/drcandacemakedamoore/cleanX Branch with paper.md (empty if default branch): Version: v0.1.14 Editor: !--editor-->@cMadan<!--end-editor-- Reviewers: @henrykironde, @anki-xyz Archive: 10.5281/zenodo.6331739

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/47ee52ff835dcd67c1f0b4c9cb74225a"><img src="https://joss.theoj.org/papers/47ee52ff835dcd67c1f0b4c9cb74225a/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/47ee52ff835dcd67c1f0b4c9cb74225a/status.svg)](https://joss.theoj.org/papers/47ee52ff835dcd67c1f0b4c9cb74225a)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@henrykironde & @anki-xyz, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @cMadan know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @henrykironde

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @anki-xyz

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @henrykironde, @anki-xyz it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 3 years ago

Wordcount for paper.md is 714

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1145/2207243.2207253 is OK
- 10.18637/jss.v059.i10 is OK
- 10.1007/978-3-319-94878-2_6 is OK
- 10.1007/s00330-020-07453-w is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.25 s (186.9 files/s, 160605.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          20            902           1684           3243
Jupyter Notebook                10              0          30961           1031
CSS                              1            192             32            685
Markdown                         5             98              0            259
YAML                             3             12             10            223
TeX                              1              8              0             57
Bourne Shell                     2              3              0             32
reStructuredText                 2             14              8             28
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            46           1241          32703           5593
-------------------------------------------------------------------------------

Statistical information for the repository '1d949d34dfc08c81a0156016' was
gathered on 2021/08/18.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Candace Makeda Moore           192         18316          15433           84.45
Oleg Sivokon                    79          4576           1630           15.53
andrew-f-murphy                  2             5              5            0.03

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Candace Makeda Moore       2143           11.7          2.0                9.24
Oleg Sivokon               3682           80.5          0.7                9.53
andrew-f-murphy               4           80.0          3.6              100.00
whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

henrykironde commented 3 years ago

I have requested some suggestions as part of the review https://github.com/drcandacemakedamoore/cleanX/issues/36 https://github.com/drcandacemakedamoore/cleanX/issues/35

cMadan commented 3 years ago

@henrykironde, thanks for the update! I've looked over discussions so far and I really appreciate that you're suggesting more robust software development principles (e.g., branches, workflows).

whedon commented 3 years ago

:wave: @anki-xyz, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @henrykironde, please update us on how your review is going (this is an automated reminder).

anki-xyz commented 3 years ago

👋 @anki-xyz, please update us on how your review is going (this is an automated reminder).

Hi, can someone resent me the invite link? It seems to be expired, thanks!

anki-xyz commented 3 years ago

@cMadan can we maybe schedule a five min zoom or something? I feel stupid to not actually can do the things I am supposed to do... I think I need some kind of help. Thank you!

cMadan commented 3 years ago

@whedon re-invite @anki-xyz as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@anki-xyz please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

cMadan commented 3 years ago

@anki-xyz, let me know if that fixes things!

anki-xyz commented 3 years ago

Things work now, thank you!

henrykironde commented 3 years ago

Here are some recommendations @drcandacemakedamoore, feel free to let me know what is appropriate to add to your software.

  1. Combine the Api source docs with other related documentation like Medical professional documentation, include Most of the sections in the README or all of it.
  2. Add developer docs. Contributors should be able to set up this development environment with minimal effort and run tests locally. Example for reference https://retriever.readthedocs.io/en/latest/developer.html
  3. Add anenvironment.yml to make it simple for users to set up the Conda virtual environment in the installation part.
  4. Move all the badges to the top. This makes it easy for users to notice the state of the software.
  5. Explain your workflow in the developer docs. (2-3 sentences)
  6. Since you are changing the main only at release, I strongly recommend that you create a branch called development for contributors to add patches before the release. Currently, you have several branches and it is hard to see any progress on the software after the release. Why the name development or dev? This is very clear for most users.
drcandacemakedamoore commented 3 years ago

Thank you very much. We will try to implement ALL of these changes, and push them next week along with some additional functions! There may be some technical issues that will come up with the .yml file, but we will try to make one as instructed.

On Thu, Sep 30, 2021 at 7:18 AM henry senyondo @.***> wrote:

Here are some recommendations @drcandacemakedamoore https://github.com/drcandacemakedamoore, feel free to let me know what is appropriate to add to your software.

  1. Combine the Api source docs with other related documentation like Medical professional documentation, include Most of the sections in the README or all of it.
  2. Add developer docs. Contributors should be able to set up this development environment with minimal effort and run tests locally. Example for reference https://retriever.readthedocs.io/en/latest/developer.html
  3. Add anenvironment.yml to make it simple for users to set up the Conda virtual environment in the installation part.
  4. Move all the badges to the top. This makes it easy for users to notice the state of the software.
  5. Explain your workflow in the developer docs. (2-3 sentences)
  6. Since you are changing the main only at release, I strongly recommend that you create a branch called development for contributors to add patches before the release. Currently, you have several branches and it is hard to see any progress on the software after the release. Why the name development or dev? This is very clear for most users.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3632#issuecomment-930792121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMYSAOIDPMLJZFR54MYBOTUEPXLHANCNFSM5CNBSWDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

wvxvw commented 3 years ago

Add an environment.yml to make it simple for users to set up the Conda virtual environment in the installation part.

@henrykironde

I started working on this, and then I realized that... well, we build the package for number of platforms (you can see them in the readme), the dependencies are different for each platform, so, it's not possible to have a single environment.yml. Even worse, we build the package for platforms neither of us has access to (MacOS).

So, if anyone wanted to develop on, say, a Mac, then they need a different environment.yml than the one I might use for Linux. I can, probably generate all of those environment files in CI, but there are two problems with that:

Alternatively, I could simply add a single file for one of my Linux environments... or I could add three, which is more realistic, since I use an environment for Python 3.7, 3.8 and 3.9. And, since Python 3.10 is coming out next week, soon we'll try to support that too. So, there will be four files, just for Linux.

Anyways, if you just want one environment file, what version of Python do you want it for?

wvxvw commented 3 years ago

@henrykironde Can you please look at this branch: https://github.com/drcandacemakedamoore/cleanX/tree/wvxvw/review-fixes and tell me if it helps you with conda environments?

henrykironde commented 3 years ago

Thanks for the Updates.

I started working on this, and then I realized that... well, we build the package for number of platforms (you can see them in the readme), the dependencies are different for each platform

Could you explain to how the dependencies are different for each platform.

Can you please look at this branch: I think that does not help me as a contributor to install from source. I would expect to see list of non python prerequisite tools. Once all are installed, it would be a clean pip install

wvxvw commented 3 years ago

We don't use pip during development. So, if you want to use it, I could try to help you, but this might be hard... The way it would work is that you'd first build the package (we don't build source packages ourselves, so, better go with Wheel, because that's something we tried). Then, you'd install the built packages. I.e. the process is:

python ./setup.py bdist_wheel
python -m pip install ./dist/*.whl

Make sure you have wheel package installed before you try to bdist_wheel.

The way to work on the project is to build the sources and then to install the resulting package. If you are using Anaconda, then you would use conda build -c conda-forge ./conda-pkg/ after you have generated the necessary configuration files. An then conda install -yqn $build_environment --use-local cleanx.

I know it's a popular practice in Python community to do something like pip install -e . in the project directory... but, years of experience taught me that while this works OK for projects developed internally and only worked on by a small group of programmers inside a single organization, once projects are made public, this no longer works due to the difference in individual setups.

This also goes to answering your question about dependencies. It... depends. Depends on what you count. The way I've generated the environment files now, they only list the essential dependencies, similar to how the build configuration does. So, you may reasonably expect them to be the same... or at least very similar. However, another, and more popular way to generate the dependency list is to run conda env export.

The later will ensure that the dependencies you have are exactly like mine (whereas in the former case, you might get newer packages installed, because that's how conda constraint solver works). However, conda lists as dependencies platform libraries like ld_impl_linux-64=2.36.1=hea4e1c9_0. If you are on a Mac, there's no way you can reproduce this setup, because it specifically wants a library built for Linux. But, it's also not necessary for you to have it on a Mac, because if you start installing from the bare minimum, the constraint solver will never even try to installing this library (because it's conditioned on operating system).

So, really, if you want to build from source, please follow what the CI script does for the platform that you have. If you find it difficult to do so, please tell me the combination of Python version, Python distribution and operating system (I assume MacOS) you want to build on, and I'll extract the CI script into something that fits your platform.

wvxvw commented 3 years ago

Actually, I just looked into environment files for Mac:

Python 3.9:

name: cleanx-env-darwin-py39
channels:
  - conda-forge
  - defaults
dependencies:
  - pandas=1.3.3=py39h4d6be9b_0
  - numpy=1.21.2=py39h7eed0ac_0
  - matplotlib=3.4.3=py39h6e9494a_1
  - pillow=8.3.2=py39he9bb72f_0
  - tesserocr=2.5.1=py39h81cd7d3_2
  - libopencv=4.5.3=py39h84599ab_2
  - pytest=6.2.5=py39h6e9494a_0
  - pycodestyle=2.7.0=pyhd8ed1ab_0
  - sphinx=4.2.0=pyh6c4a22f_0

Python 3.8

name: cleanx-env-darwin-py38
channels:
  - conda-forge
  - defaults
dependencies:
  - pandas=1.3.3=py38ha53d530_0
  - numpy=1.21.2=py38h49b9922_0
  - matplotlib=3.4.3=py38h50d1736_1
  - pillow=8.2.0=py38h83525de_1
  - tesserocr=2.5.1=py38h0a5c65b_2
  - libopencv=4.5.0=py38_3
  - pytest=6.2.5=py38h50d1736_0
  - pycodestyle=2.7.0=pyhd8ed1ab_0
  - sphinx=4.2.0=pyh6c4a22f_0

So, as you can see, they are totally different because of Python version. It's not just a tiny difference. These are entirely different packages.

henrykironde commented 3 years ago

Thanks for the explanation @wvxvw, I still don't see the difference. Could you clarify that for me, please?

Also, I have looked through the development docs of core packages that you are using, and they use pip install. pandas https://github.com/pandas-dev/pandas numpy https://numpy.org/devdocs/dev/development_environment.html#other-build-options matplotlib https://matplotlib.org/stable/users/installing.html pillow https://pillow.readthedocs.io/en/latest/installation.html#basic-installation I have personally installed these packages locally at some time when debugging some of the software projects I maintain. Please, compare your installation guide to what your dependency packages have, and come up with some docs that enable developers to utilize your tool with ease.

wvxvw commented 3 years ago

I still don't see the difference. Could you clarify that for me, please?

Look at the part that follows the second equals sign, it tells you the package was built for a different version of Python. Every minor version of Python has different ABI. In other words, if you link against libpython, you must link against a specific version of it. You cannot have a version of a package that can be loaded by either Python 3.8 or 3.9, if it has a shared library that needs to link with libpython. Not only that, Python runtime reserves the right to change bytecode format between minor version, that's why Wheel format requires that you specify the Python's minor version in the name of the Wheel, if you are packaging bytecompiled Python files or shared libraries linked against libpython.

Also, I have looked through the development docs of core packages that you are using, and they use pip install.

How they manage their development process is up to them... I believe, you, of course, realize that if you use pip install to install your package, this doesn't help you to deal with Anaconda's packages. So, if you want to develop for Anaconda, you need a different toolchain.

Please, compare your installation guide to what your dependency packages have, and come up with some docs that enable developers to utilize your tool with ease.

At the moment we don't have an installation guide for developers. We only instruct the end users on how to install the package.

I will certainly add instructions for developers to the readme, but the ease is in the eye of the beholder. I'm still waiting for your response on the branch I've sent you. Does it help you? If not, why, and what can be improved? Please help me understand what parts don't you find easy.

Another question, that I would really like you to answer to help me understand your difficulties is whether you want to work on our project in the same way we work on it, or do you want us to make your working style possible with our project. I.e. I understand that your working style relies on pip install -e ., if you develop for PyPI and on conda create -f env.yaml if you develop for Anaconda. Am I correct about that?

Furthermore, we don't have any formal or informal process for accepting contributions from developers who aren't on our team. We are too few, and didn't anticipate the need for such a thing. So, we don't have any requirements for developers, and I believe the developers may use whatever tools they feel most convenient with to work on the project. I'm not sure that hand-holding them to create their working environment is my place.

henrykironde commented 3 years ago

Another question, that I would really like you to answer to help me understand your difficulties is whether you want to work on our project in the same way we work on it, or do you want us to make your working style possible with our project. I.e. I understand that your working style relies on pip install -e ., if you develop for PyPI and on conda create -f env.yaml if you develop for Anaconda. Am I correct about that?

What I am trying to say is that there are standards in software development, these standards need to be maintained by all users for the ecosystem to have fewer dependency problems. I did not create these standards, and that is why I pointed you to the docs of the packages you are using, to have some points of reference. This will also avoid backward compatibility issues where a package does not depend on a version as Python 3.8 or 3.9 to work.

cleanX package is a pure Python package It should be able to run on the development tools in the core Python ecosystem, otherwise you can state the difference that it only runs in a Conda environment.

I think at this moment, I have provided enough recommendations to improve the utility of the software, and the quality to my expectations of the "industrial" standard. Feel free to make only the changes you think are appropriate. I will review the software when you feel ready.

wvxvw commented 3 years ago

This will also avoid backward compatibility issues where a package does not depend on a version as Python 3.8 or 3.9 to work.

I'm sorry... you misinterpreted what I wrote. Please, look again at the files generated for environments using different versions of Python. It's impossible to have the same set of dependencies because of how Python works. It's not my choice, or the choice of Pandas or NumPy or any other package.

The development instructions they (Pandas, NumPy etc.) give you are the instructions for a version of Python. Maybe the latest supported by the given package, or maybe the latest supported by the released version of the package... they don't give you a way to set up your working environment for all versions.

If you want instructions for Python 3.9, MacOS, Anaconda Python, that is something I can give you. But, I cannot give you, for example, instructions on how to set up your environment if you want to work with Python 3.9, MacOS, python.org's Python, simply because we don't support this combination of platforms. I've never tried to use the package there, and have no access to the platform to even attempt to use it there.

You also misunderstand the backwards compatibility problem (in the context of our package). We are not developing for a particular version of Python, treating all older versions as backwards (in-)compatible. We develop for different versions of Python. From our perspective, they are all valid and valuable. As such, we don't have a "main" version, that aligns with a particular Python version, and "soon to be obsolete" versions that need less development effort and that soon will be abandoned.

I think at this moment, I have provided enough recommendations to improve the utility of the software, and the quality to my expectations of the "industrial" standard.

Then I will treat the bullet point 3 in your initial post as being addressed by the branch wvxvw/review-fixes. It still needs some polishing of documentation, but the main feature added there is my response to your request. If you are not satisfied by the changes, please feel free to revisit it.

wvxvw commented 3 years ago

Actually, the same branch also closes the bullet point 5 on your list.

As for bullet point 6: our development process doesn't require it, but I will add it anyways. If you are planning on making a contribution, please read the documentation added in wvxvw/review-fixes.

drcandacemakedamoore commented 3 years ago

We have now tried to accomplish all the bullet points. For me it meant changing to a new workflow so that main is the stable released version only, which will almost definitely help us in the long run. Thank you for your help, and we will try to accomplish any other requests.

On Sun, Oct 3, 2021 at 10:55 AM wvxvw @.***> wrote:

Actually, the same branch also closes the bullet point 5 on your list.

As for bullet point 6: our development process doesn't require it, but I will add it anyways. If you are planning on making a contribution, please read the documentation added in wvxvw/review-fixes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3632#issuecomment-932891820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMYSAKGAALVVCOFYLMPZX3UFAK6LANCNFSM5CNBSWDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cMadan commented 2 years ago

@henrykironde @anki-xyz, can you give me an update on how the review is going? Thank you!

henrykironde commented 2 years ago

@cMadan thank you for the reminder. I have constantly checked on the progress of the software submitted however, I still can not get the source to install. Current error messages am getting :

Traceback (most recent call last):
  File "/opt/miniconda3/envs/cleanx/bin/conda-build", line 11, in <module>
    sys.exit(main())
  File "/opt/miniconda3/envs/cleanx/lib/python3.9/site-packages/conda_build/cli/main_build.py", line 481, in main
    execute(sys.argv[1:])
  File "/opt/miniconda3/envs/cleanx/lib/python3.9/site-packages/conda_build/cli/main_build.py", line 470, in execute
    outputs = api.build(args.recipe, post=args.post, test_run_post=args.test_run_post,
  File "/opt/miniconda3/envs/cleanx/lib/python3.9/site-packages/conda_build/api.py", line 184, in build
    raise ValueError('No valid recipes found for input: {}'.format(recipe_paths_or_metadata))
ValueError: No valid recipes found for input: ['/Users/henrysenyondo/Downloads/cleanX/conda-pkg']
Traceback (most recent call last):
  File "./setup.py", line 448, in <module>
    setup(
  File "/opt/miniconda3/lib/python3.8/site-packages/setuptools/__init__.py", line 161, in setup
    return distutils.core.setup(**attrs)
  File "/opt/miniconda3/lib/python3.8/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/opt/miniconda3/lib/python3.8/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/opt/miniconda3/lib/python3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "./setup.py", line 404, in run
    super().run()
  File "./setup.py", line 348, in run
    raise RuntimeError('Couldn\'t build {} package'.format(name))
RuntimeError: Couldn't build cleanX package

Additionally the whole documentation of the package needs improvement. The instructions are not very clear and need better organization. I recommend the authors to go through the instructions provided and test them out. An example 

conda create -n cleanx
conda activate -n cleanx.  # will error
python ./setup.py install_dev

Another example:

Whether python.org Python or Anaconda Python are supported, it means that version 3.7, 3.8 and 3.9 are supported. We know for certain that 3.6 is not supported, and there will be no support in the future.

I believe the author means: Supports 3.7+.
(Simplicity is better and easy to read)

wvxvw commented 2 years ago

@henrykironde My apologies, the installation instructions were incomplete. I've updated the documentation to reflect that:

conda create -n cleanx
conda activate cleanx
python ./setup.py genconda
python ./setup.py install_dev

This is the right sequence of commands. In addition, about setup.py genconda, this command generates necessary Anaconda package configuration files, unfortunately, they are specific to Python version. Unfortunately, right now, it's not possible to have multiple configurations concurrently (but it may be fixed in the future, if you really find this inconvenient). This means, that if you switch between different versions of Python, you will have to re-run this command.

Additionally, I've noticed that you are using MacOS and a combination of Python 3.8 and Python 3.9. To be honest, I didn't know that was even possible. I.e. you seem to be running conda command with Python 3.8, but the conda packages themselves reside in Python 3.9 installation... As we don't try this in our ordinary development, I cannot guarantee that your setup will work. Please try using a single version of Python if at all possible.

On top of this, since you are using MacOS, some time ago we've discovered that in the combination of MacOS and Python 3.9 you will not be able to get OpenCV to work. I'm not 100% sure at the moment what the issue was, but, I believe, it's related to XCode and some clang flags. Since it looks like you have Python 3.8 available, I'd recommend using that instead.

Thank you for bringing the problems with installation sequence to our attention. Dr. Moore is dealing with other aspects of your report, i.e. documentation.

henrykironde commented 2 years ago

Thank you for the nice feedback @wvxvw. So I have tested this on two machines. One one the genconda command worked so well but failed on the other. I am getting the following error.

Mac OS : 

(cleanx) ➜  cleanX git:(main) ✗ python ./setup.py genconda
Traceback (most recent call last):
  File "./setup.py", line 503, in <module>
    zip_safe=False,
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/__init__.py", line 144, in setup
    _install_setup_requires(attrs)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/__init__.py", line 139, in _install_setup_requires
    dist.fetch_build_eggs(dist.setup_requires)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/dist.py", line 717, in fetch_build_eggs
    replace_conflicting=True,
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 782, in resolve
    replace_conflicting=replace_conflicting
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 1065, in best_match
    return self.obtain(req, installer)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 1077, in obtain
    return installer(requirement)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/dist.py", line 784, in fetch_build_egg
    return cmd.easy_install(req)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/command/easy_install.py", line 679, in easy_install
    return self.install_item(spec, dist.location, tmpdir, deps)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/command/easy_install.py", line 705, in install_item
    dists = self.install_eggs(spec, download, tmpdir)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/command/easy_install.py", line 890, in install_eggs
    return self.build_and_install(setup_script, setup_base)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/command/easy_install.py", line 1158, in build_and_install
    self.run_setup(setup_script, setup_base, args)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/command/easy_install.py", line 1144, in run_setup
    run_setup(setup_script, args)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 253, in run_setup
    raise
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 195, in setup_context
    yield
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 166, in save_modules
    saved_exc.resume()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 141, in resume
    six.reraise(type, exc, self._tb)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 154, in save_modules
    yield saved
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 195, in setup_context
    yield
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 250, in run_setup
    _execfile(setup_script, ns)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/setuptools/sandbox.py", line 45, in _execfile
    exec(code, globals, locals)
  File "/var/folders/rg/t6xt8whj6cx1yt3tn741j8740000gn/T/easy_install-UJOKXd/Sphinx-4.2.0/setup.py", line 8, in <module>
  File "/var/folders/rg/t6xt8whj6cx1yt3tn741j8740000gn/T/easy_install-UJOKXd/Sphinx-4.2.0/sphinx/__init__.py", line 20, in <module>
  File "/var/folders/rg/t6xt8whj6cx1yt3tn741j8740000gn/T/easy_install-UJOKXd/Sphinx-4.2.0/sphinx/deprecation.py", line 28
    def deprecated_alias(modname: str, objects: Dict[str, object],
                                ^
SyntaxError: invalid syntax

on the machine that runs genconda well I get the following error in the python setup install_dev

UnsatisfiableError: The following specifications were found
to be incompatible with the existing python installation in your environment:

Specifications:

  - cleanx -> python==3.8

Your python: python=3.9

If python is on the left-most side of the chain, that's the version you've asked for.
When python appears to the right, that indicates that the thing on the left is somehow
not available for the python version you are constrained to. Note that conda will not
change your python version to a different minor version unless you explicitly specify
that.

Traceback (most recent call last):
  File "./setup.py", line 448, in <module>
    setup(
  File "/opt/miniconda3/lib/python3.8/site-packages/setuptools/__init__.py", line 161, in setup
    return distutils.core.setup(**attrs)
  File "/opt/miniconda3/lib/python3.8/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/opt/miniconda3/lib/python3.8/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/opt/miniconda3/lib/python3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "./setup.py", line 404, in run
    super().run()
  File "./setup.py", line 360, in run
    raise RuntimeError('Couldn\'t install {} package'.format(name))
RuntimeError: Couldn't install cleanX package
henrykironde commented 2 years ago

Somethings that I noted, like you said, we should not have two references for python 3.8 and 3.9. I guess we should try to create the env stating the python version to run the package. That may probably resolve some issue. Again thanks for the comments.

drcandacemakedamoore commented 2 years ago

Is it possible that you downloaded Anaconda for Python 3.8, then upgraded to Anaconda (which might give you one for Python 3.9)? Anaconda can apparently sometimes upgrade itself without you asking it to (we just discovered this today, and it actually gave us the same problem). If so I think you could downgrade, or do a clean install of Anaconda. As we now have the same problem, I expect Oleg will write more details on how we can fix it.

On Wed, Nov 3, 2021 at 8:19 PM henry senyondo @.***> wrote:

Somethings that I noted, like you said, we should not have two references for python 3.8 and 3.9. I guess we should try to create the env stating the python version to run the package. That may probably resolve some issue. Again thanks for the comments.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3632#issuecomment-959846153, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHMYSAMNOCA3RSEOXDCQRPDUKGKMLANCNFSM5CNBSWDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

wvxvw commented 2 years ago

Fix is on the way to develop.

If you are interested to know what happened, here's a synopsis:

conda install xxx will ignore Python version you configured for your virtual environment, and will install arbitrary version it likes, and if it means that in order to install this version conda needs to install a different version of Python, or a different version of conda etc, then so be it.

It didn't always work like that. But it did at the time of Python 2.7. Then, at some point the behavior improved, but at some later point the bad behavior returned. In the latest version of conda, the behavior described above is what you get.

So, why it worked just a week ago and why it doesn't work today: in order to build our package we install conda-build and conda-verify. We don't require a specific version of either of those packages as it's irrelevant to us. However, even though you probably created a virtual environment using Python 3.8, conda discovered conda-build=3.21.5=py39h06a4308_0 (or similar, if you are on a Mac / Windows). Even though there's conda-build-3.21.5=py38h06a4308_0, conda decided that it's better to update everything in your environment... including Python and conda itself. It must be the case that conda-build was unavailable for Python 3.9, and so the newer version existed either solely in 3.7 or maybe conda in 3.7 wouldn't update Python and the latest version of conda-build was from 3.8.

In our build, we request specifically to build our package for the version of Python that was installed in your environment, i.e. 3.8. Because build happens in a separate environment (I don't know how to circumvent this, it's completely unnecessary and slows the build by a lot), the package was built successfully, but for the wrong Python version.

Worse yet, conda ignored Python requirement when testing the package (I've also tried to add it explicitly to the test section of the meta.yaml -- it has no effect. And that's how you ended up with the error you have.

andrew-f-murphy commented 2 years ago

Hey, @henrykironde we've gone through and cleaned up language and typos. Do you require anything else from our end?

henrykironde commented 2 years ago

I still cannot get this package to install. Could someone from the team be free to have a thorough walk through meeting. I am okey with whatever time works for any of the members.

wvxvw commented 2 years ago

Hi @henrykironde what time zone are you in? I'm not sure about Monday and Tuesday due to a family emergency, but after that, I'll try to help you.

henrykironde commented 2 years ago

ES, any time works for me, night or day.

wvxvw commented 2 years ago

Hi, Henry, sorry it's taking longer than I hoped.

How about 13:00 UTC on Saturday? If that's inconvenient, then maybe 20:00 UTC on the same day, or it could be an hour or two later.

On Mon, Mar 14, 2022 at 12:09 AM henry senyondo @.***> wrote:

ES, any time works for me, night or day.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3632#issuecomment-1066202075, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYAMDDFTAB3MP44UX5KMX3U7Z7UHANCNFSM5CNBSWDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

henrykironde commented 2 years ago

20:00 UTC works fine

wvxvw commented 2 years ago

Hi, sorry, @henrykironde I realized I don't know how to contact you :)

wvxvw commented 2 years ago

OK, I think, I figured out how to use GMail video calls.

henrykironde commented 2 years ago

@wvxvw I sent you an email from henrykironde@gmail.com. We can reschedule for another time.

andrew-f-murphy commented 2 years ago

@henrykironde Any luck on getting the package to install? i know @wvxvw has a lot on at the moment so we could be a while sorry.

henrykironde commented 2 years ago

@whedon generate pdf

editorialbot commented 2 years ago

My name is now @editorialbot

henrykironde commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

andrew-f-murphy commented 2 years ago

@henrykironde Not sure what is happening with the author post nominals I know mine should read, Andrew Murphy, BMedImagingSc, MMIS, RT(R). Trying to find the others but am on a plane and have spotty recption