caracal-pipeline / caracal

Containerized Automated Radio Astronomy Calibration (CARACal) pipeline
GNU General Public License v2.0
28 stars 6 forks source link

Fix #1510 #1513

Closed KshitijT closed 1 year ago

KshitijT commented 1 year ago

Implementing @o-smirnov and @PeterKamphuis 's suggestion to fix #1510.

Summary:

Added "GainDiagAmp": 'complex-2x2', at line 42 in the selfcal worker.

o-smirnov commented 1 year ago

@Athanaseus or @SpheMakh, looks like flake itself is failing on Python 3.7. But we don't support it anymore so why don't we retire it?

paoloserra commented 1 year ago

@KshitijT the PR test is failing for python 3.7, but as pointed out by @PeterKamphuis at the telecon caracal now requires python 3.8 or higher

Athanaseus commented 1 year ago

Support for py3.7 was reincorporated in #1498.

[tool.poetry.group.tests.dependencies]
-flake8 = "*"
+flake8 = "^5.0.0"

[tool.poetry.group.docs.dependencies]
-Sphinx = "^5.3.0"
+Sphinx = "^4.0.1"

The following sphinx and flake8 libraries satisfy all the python versions [3.7, 3.8, 3.9, 3.10]

@KshitijT please make the above changes in pyproject.toml Then pip install poetry && poetry lock to update the lock file of requirements.

KshitijT commented 1 year ago

Support for py3.7 was reincorporated in #1498.

[tool.poetry.group.tests.dependencies]
-flake8 = "*"
+flake8 = "^5.0.0"

[tool.poetry.group.docs.dependencies]
-Sphinx = "^5.3.0"
+Sphinx = "^4.0.1"

The following sphinx and flake8 libraries satisfy all the python versions [3.7, 3.8, 3.9, 3.10]

@KshitijT please make the above changes in pyproject.toml Then pip install poetry && poetry lock to update the lock file of requirements.

@Athanaseus , thanks for the clarification. One quick question, is this already included in the master? Because the branch the PR is based on is fairly new one.

Athanaseus commented 1 year ago

Support for py3.7 was reincorporated in #1498.

[tool.poetry.group.tests.dependencies]
-flake8 = "*"
+flake8 = "^5.0.0"

[tool.poetry.group.docs.dependencies]
-Sphinx = "^5.3.0"
+Sphinx = "^4.0.1"

The following sphinx and flake8 libraries satisfy all the python versions [3.7, 3.8, 3.9, 3.10] @KshitijT please make the above changes in pyproject.toml Then pip install poetry && poetry lock to update the lock file of requirements.

@Athanaseus , thanks for the clarification. One quick question, is this already included in the master? Because the branch the PR is based on is fairly new one.

No, for this reason, the master is not stable within python3.7. You need to make the changes, otherwise I'm happy to make a separate PR that addresses this and we can merge it, then update this one.

KshitijT commented 1 year ago

Support for py3.7 was reincorporated in #1498.

[tool.poetry.group.tests.dependencies]
-flake8 = "*"
+flake8 = "^5.0.0"

[tool.poetry.group.docs.dependencies]
-Sphinx = "^5.3.0"
+Sphinx = "^4.0.1"

The following sphinx and flake8 libraries satisfy all the python versions [3.7, 3.8, 3.9, 3.10] @KshitijT please make the above changes in pyproject.toml Then pip install poetry && poetry lock to update the lock file of requirements.

@Athanaseus , thanks for the clarification. One quick question, is this already included in the master? Because the branch the PR is based on is fairly new one.

No, for this reason, the master is not stable within python3.7. You need to make the changes, otherwise I'm happy to make a separate PR that addresses this and we can merge it, then update this one.

Cool, let me make the changes in the PR. I'll mark you as a reviewer on this PR, please check if the changes are satisfactory once I am done.

Athanaseus commented 1 year ago

Roger that!

Athanaseus commented 1 year ago

pip install poetry && poetry lock to update the lock file of requirements. commit the updated poetry.lock

KshitijT commented 1 year ago

pip install poetry && poetry lock to update the lock file of requirements. commit the updated poetry.lock

Just to clarify, I should add this line: pip install poetry && poetry lock in the poetry.lock file?

Athanaseus commented 1 year ago

Running that command in your development environment should auto-update the https://github.com/caracal-pipeline/caracal/blob/master/poetry.lock file. It would help if you committed that too.

Athanaseus commented 1 year ago

@KshitijT please re-check this, I updated the lock file as required. I also hijacked this PR to update readme (resolves #1519)

KshitijT commented 1 year ago

@Athanaseus, is there something in particular you would like me to check for this PR?

Athanaseus commented 1 year ago

@KshitijT No, it's ready for merge.

KshitijT commented 1 year ago

@KshitijT No, it's ready for merge.

Ok, then let's just wait until the checks pass.

KshitijT commented 1 year ago

@Athanaseus do we wait for the 3.6-5.7-3.8 builds to go through or they are just stuck and we should just merge?