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

Allow geometry-less models #285

Merged
merged 1 commit into from Mar 21, 2024
Merged

Allow geometry-less models #285

merged 1 commit into from Mar 21, 2024

Conversation

3nids
Copy link
Contributor

@3nids 3nids commented Sep 27, 2023

A null geometry is returned in such case

Fixes #282

Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

thanks for the patch! this will need additional test as well. something like using the code examples from the issue you reported.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @3nids, we need at least 1 test for this and a 1 lines of text to explain its usage in the README.

Please also check the build failure to see how to fix those QA issues.

@3nids
Copy link
Contributor Author

3nids commented Sep 28, 2023

Thank for the feedback!
I added some tests.
May I ask why running the test workflow requires an approval? It would ease the process to have a quicker feedback.
Cheers.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@3nids keep in mind the commit message QA checks run on each commit, you can run those locally. with ./run-qa-checks

@auvipy
Copy link
Collaborator

auvipy commented Oct 8, 2023

api.py", line 150, in pass_env_post_process
raise Fail(msg)
tox.tox_env.errors.Fail: pass_env values cannot contain whitespace, use comma to have multiple values in a single line, invalid values found 'TRAVIS TRAVIS_*'
Error: Process completed with exit code 1.

@3nids 3nids force-pushed the patch-1 branch 2 times, most recently from 3b640c6 to 1cb9c89 Compare October 10, 2023 05:32
@3nids
Copy link
Contributor Author

3nids commented Oct 10, 2023

I made something wrong with my last force push, trying again.

@auvipy
Copy link
Collaborator

auvipy commented Oct 10, 2023

hope this will be green this time!

@auvipy auvipy self-requested a review October 10, 2023 06:20
@3nids
Copy link
Contributor Author

3nids commented Oct 10, 2023

Looks like I didn't run makemigrations.
hopefully I'll be back...

@3nids

This comment was marked as outdated.

@3nids

This comment was marked as outdated.

@3nids 3nids force-pushed the patch-1 branch 3 times, most recently from 6109610 to 14eae96 Compare October 18, 2023 06:13
@3nids

This comment was marked as outdated.

@3nids

This comment was marked as outdated.

@auvipy
Copy link
Collaborator

auvipy commented Oct 28, 2023

Done

Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

Can you please check why this error is occuring?

tox -e py39-django32-djangorestframework312
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.9.18/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.9.18/x64/lib
POSTGRES_HOST: localhost
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.9.18/x64/bin/tox", line 8, in
sys.exit(run())
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/run.py", line 19, in run
result = main(sys.argv[1:] if args is None else args)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/run.py", line 45, in main
return handler(state)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/cmd/legacy.py", line 115, in legacy
return run_sequential(state)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/cmd/run/sequential.py", line 24, in run_sequential
return execute(state, max_workers=1, has_spinner=False, live=True)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/cmd/run/common.py", line 236, in execute
state.envs.ensure_only_run_env_is_active()
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/env_select.py", line 416, in ensure_only_run_env_is_active
envs, active = self._defined_envs, self._env_name_to_active()
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/env_select.py", line 273, in _defined_envs
raise failed[next(iter(failed_to_create))]
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/env_select.py", line 250, in _defined_envs
run_env.package_env = self._build_pkg_env(pkg_name_type, name, env_name_to_active)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/session/env_select.py", line 321, in _build_pkg_env
name_type = next(child_package_envs)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/package/pyproject.py", line 173, in register_run_env
yield from super().register_run_env(run_env)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/package.py", line 116, in register_run_env
pkg_env = run_env.conf["wheel_build_env"]
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/sets.py", line 118, in getitem
return self.load(item)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/sets.py", line 129, in load
return config_definition.call(self._conf, self.loaders, ConfigLoadArgs(chain, self.name, self.env_name))
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/of_type.py", line 105, in call
value = self.default(conf, args.env_name) if callable(self.default) else self.default
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/package.py", line 92, in default_wheel_tag
run_py = cast(Python, run_env).base_python
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/api.py", line 250, in base_python
self._base_python = self._get_python(base_pythons)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/api.py", line 134, in _get_python
interpreter = self.creator.interpreter
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/api.py", line 126, in creator
return self.session.creator
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/api.py", line 106, in session
env = self.virtualenv_env_vars()
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/api.py", line 111, in virtualenv_env_vars
env = self.environment_variables.copy()
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/python/virtual_env/api.py", line 166, in environment_variables
environment_variables = super().environment_variables
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/runner.py", line 193, in environment_variables
environment_variables = super().environment_variables
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/api.py", line 330, in environment_variables
pass_env: list[str] = self.conf["pass_env"]
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/sets.py", line 118, in getitem
return self.load(item)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/sets.py", line 129, in load
return config_definition.call(self.conf, self.loaders, ConfigLoadArgs(chain, self.name, self.env_name))
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/config/of_type.py", line 107, in call
value = self.post_process(value)
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/tox/tox_env/api.py", line 150, in pass_env_post_process
raise Fail(msg)
tox.tox_env.errors.Fail: pass_env values cannot contain whitespace, use comma to have multiple values in a single line, invalid values found 'TRAVIS TRAVIS
*'
Error: Process completed with exit code 1.

@3nids
Copy link
Contributor Author

3nids commented Oct 30, 2023

I haven't touched anything in tox.ini, quite strange.
But I was wondering why there was some reference to TRAVIS variables in it? remains from the past? Maybe this needs to be dropped?

@3nids 3nids closed this Oct 30, 2023
@3nids 3nids reopened this Oct 30, 2023
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@3nids please try rebasing on the latest master to get rid of some errors.

@3nids
Copy link
Contributor Author

3nids commented Oct 30, 2023

done, thanks!

@3nids 3nids requested a review from nemesifier October 31, 2023 13:39
@3nids

This comment was marked as outdated.

@3nids

This comment was marked as outdated.

@nemesifier
Copy link
Member

Triggered @3nids

@3nids
Copy link
Contributor Author

3nids commented Mar 4, 2024

@nemesifier thanks to the help from @suricactus this should be good now. That'd be great to re trigger the tests! Cheers!

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

It's good to see progress on this! Ran the build, 1 test failing, please check. See also my suggestion below.

processed_fields.add(self.Meta.geo_field)
# geometry attribute
# must be present in output according to GeoJSON spec
if self.Meta.geo_field:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if self.Meta.geo_field:
if self.Meta.geo_field is not None:

Choose a reason for hiding this comment

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

I think this is good like this, we want to ignore stuff like "" or 0 that ended up here by accident.

@suricactus
Copy link

@3nids this makes all tests to pass locally. Forgot to commit last time: 3nids#2

@3nids
Copy link
Contributor Author

3nids commented Mar 5, 2024

@nemesifier thanks again, should be good this time 🤞

@3nids

This comment was marked as outdated.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thx for the reminder, I restarted the build, there still seems to be one test failing.

@@ -118,7 +123,7 @@ def to_representation(self, instance):
processed_fields = set()

# optional id attribute
if self.Meta.id_field:
if self.Meta.id_field is not None:

Choose a reason for hiding this comment

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

Suggested change
if self.Meta.id_field is not None:
if self.Meta.id_field:

This somehow slipped in the force-pushed commit. In the PR I quoted earlier 3nids#2, this line is not modified and is as in the suggestion.

Note this is required by the exact test that is failing, namely test_geojson_false_id_attribute_slug which tests when the id_field = False. In that case having a strict check like this self.Meta.id_field is not None will fail and cause the serializer to search for field named False.

@3nids
Copy link
Contributor Author

3nids commented Mar 19, 2024

@nemesifier @suricactus I just dropped the 2 is not None and pushed.
Could you retrigger the tests?
cheers!

@3nids
Copy link
Contributor Author

3nids commented Mar 21, 2024

@nemesifier gentle ping :)

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thanks @auvipy @suricactus 👍 👏

@nemesifier nemesifier merged commit 4f244d5 into openwisp:master Mar 21, 2024
22 of 24 checks passed
@3nids 3nids deleted the patch-1 branch April 3, 2024 14:12
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.

Use GeoFeatureModelSerializer without geometry
4 participants